Store: Fix ESLint errors in WCS, tests, lib, and components directories #24571

justinshreve · 2018-04-30T17:44:18Z

This fixes the eslint issues in the following files:

client/extensions/woocommerce/components/bulk-select/index.js (2 err, 0 warn)
client/extensions/woocommerce/components/form-click-to-edit-input/index.js (2 err, 0 warn)
client/extensions/woocommerce/components/form-location-select/countries.js (2 err, 0 warn)
client/extensions/woocommerce/components/location-flag/index.js (1 err, 0 warn)
client/extensions/woocommerce/lib/generate-variations/test/index.js (1 err, 0 warn)
client/extensions/woocommerce/state/data-layer/ui/products/index.js (3 err, 0 warn)
client/extensions/woocommerce/state/sites/payment-methods/test/reducer.js (1 err, 0 warn)
client/extensions/woocommerce/state/sites/review-replies/test/handlers.js (1 err, 0 warn)
client/extensions/woocommerce/state/sites/reviews/test/handlers.js (1 err, 0 warn)
client/extensions/woocommerce/state/ui/payments/methods/test/selectors.js (1 err, 0 warn)
client/extensions/woocommerce/state/ui/shipping/zones/locations/test/selectors.js (1 err, 0 warn)
client/extensions/woocommerce/state/ui/shipping/zones/methods/selectors.js (2 err, 0 warn)
client/extensions/woocommerce/woocommerce-services/views/label-settings/label-settings.js (1 err, 0 warn)
client/extensions/woocommerce/woocommerce-services/views/packages/edit-package.js (1 err, 0 warn)
client/extensions/woocommerce/woocommerce-services/views/shipping-label/label-item.js (3 err, 0 warn)
client/extensions/woocommerce/woocommerce-services/views/shipping-label/label-purchase-modal/address-step/suggestion.js (3 err, 0 warn)
client/extensions/woocommerce/woocommerce-services/views/shipping-label/label-purchase-modal/packages-step/list.js (2 err, 0 warn)

Some of these are around duplicate test descriptions, indentation, and buttons vs links. No functionality should be changed so testing can just be spot checking.

To Test:

Run tests npm run test-client client/extensions/woocommerce
Click around store. Specifically, I've touched the following pages (via components or selectors):
- SKU Edit Component
- Address Dialog (Country/State Selection)
- Shipping zone & method dialogs
- Product reviews & review replies
- Label & package dialogs and flow

matticbot · 2018-04-30T17:44:20Z

Test live: https://calypso.live/?branch=fix/eslint-store-wcs-test-comp

ryelle

Just a few tweaks requested, but otherwise looks good 🙂

ryelle · 2018-05-01T16:44:54Z

client/extensions/woocommerce/components/form-location-select/countries.js

@@ -98,6 +98,20 @@ class FormCountrySelectFromApi extends Component {
 	}
 }

+const getLocationsListFromContinents = ( state, continents, siteId ) => {


I don't think this is the PR for it, but this should probably be moved to a selector in sites/data/locations, maybe called getContinentsWithCountries.

ryelle · 2018-05-01T16:49:45Z

client/extensions/woocommerce/state/ui/shipping/zones/methods/selectors.js

@@ -127,12 +127,15 @@ const overlayShippingZoneMethods = ( state, zone, siteId, extraEdits ) => {
 			enabled = getShippingZoneMethod( state, method._originalId, siteId ).enabled;
 		}

+		// Prettier formatting indents the schema line in a way eslint doesn't like.


Prettier seems to format it as following, which isn't super intuitive IMO but is valid for the linter. I think we should either update this comment, or use the prettier/eslint format

const defaultValues = startsWith( method.methodType, 'wc_services' ) ? getDefaultSettingsValues( getShippingMethodSchema( state, method.methodType, siteId ).formSchema ) : {};

ryelle · 2018-05-01T17:17:36Z

...ommerce/woocommerce-services/views/shipping-label/label-purchase-modal/packages-step/list.js

@@ -42,12 +43,13 @@ const PackageList = ( props ) => {
 		const onOpenClick = () => props.openPackage( orderId, siteId, pckgId );
 		return (
 			<div className="packages-step__list-item" key={ pckgId }>
-				<div
+				<Button
+					borderless


Now that this is a button, the contrast is a bit light:

Can you use the $gray-text color for this?

… and the purchase label dialog

justinshreve · 2018-05-02T15:30:05Z

@ryelle Thanks for the review.

I updated the text color for the packages dialog. I also added a TODO to make that a proper selector which I can handle in a follow-up PR.

For the indent/linter one -- I've tried formatting that line a few different ways. Prettier keeps updating it on commit to:

		const defaultValues = startsWith( method.methodType, 'wc_services' )
			? getDefaultSettingsValues(
					getShippingMethodSchema( state, method.methodType, siteId ).formSchema
				)
			: {};

which is slightly different from what you pasted above.

The linter will complain "Expected indentation of 4 tabs but found 5" without disabling the line there. I've at least updated the comment to be a bit clearer.

blowery · 2018-05-02T18:38:13Z

@justinshreve I wonder if #24551 would help there at all.. /cc @jsnajdr

blowery · 2018-05-02T18:40:07Z

client/extensions/woocommerce/components/form-location-select/countries.js

+// https://github.com/Automattic/wp-calypso/pull/24571#discussion_r185268996
+const getContinentsWithCountries = ( state, continents, siteId ) => {
+	const locationsList = [];
+	continents.forEach( continent => {


this could be a map instead a forEach with pushes

Thanks! In this PR I'm just going to move it out of connect, but I plan on making this a proper selector + add some tests in a follow-up, so I can do some code cleanup there.

blowery · 2018-05-02T18:58:10Z

@justinshreve #24635 should help with the weird formatting cases.

jsnajdr · 2018-05-02T19:28:16Z

I wonder if #24551 would help there at all.

The indenting of ternaries isn't a bug, it's a difference of opinion between Prettier and ESLint. The ESLint indent rule doesn't like what Prettier (intentionally) does, that's all. Prettier upgrade won't change that.

blowery · 2018-05-02T19:29:25Z

@jsnajdr #24635 should resolve that difference of opinion. :)

ryelle · 2018-05-02T19:48:11Z

@justinshreve apparently I had a setting on in my editor that ran eslint --fix after formatting, so when I used Atom to format the file, it did so with the eslint indentation... but when I ran prettier from CLI it indents like you said. Sorry about any confusion from my comment 😣

blowery

LGTM!

justinshreve added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Store labels Apr 30, 2018

justinshreve self-assigned this Apr 30, 2018

justinshreve requested a review from a team April 30, 2018 17:44

blowery mentioned this pull request Apr 30, 2018

Fix all the eslint errors #24504

Closed

ryelle reviewed May 1, 2018

View reviewed changes

justinshreve added 3 commits May 2, 2018 11:01

Fix eslint errors for WCS, components, lib, and state directories

f536a97

Fix shipping zone method indent error

193ebc3

Fix remaining errors with address selection, click to edit component,…

749a1dc

… and the purchase label dialog

justinshreve force-pushed the fix/eslint-store-wcs-test-comp branch from e1aabf7 to 749a1dc Compare May 2, 2018 15:02

Handle PR feedback

8a40cf0

blowery reviewed May 2, 2018

View reviewed changes

blowery mentioned this pull request May 2, 2018

Lint: Disable errors for formatting handled by prettier #24635

Merged

blowery approved these changes May 2, 2018

View reviewed changes

timmyc added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 2, 2018

justinshreve merged commit 1a1a65e into master May 3, 2018

matticbot removed the [Status] Ready to Merge label May 3, 2018

justinshreve deleted the fix/eslint-store-wcs-test-comp branch May 3, 2018 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store: Fix ESLint errors in WCS, tests, lib, and components directories #24571

Store: Fix ESLint errors in WCS, tests, lib, and components directories #24571

justinshreve commented Apr 30, 2018

matticbot commented Apr 30, 2018

ryelle left a comment

ryelle May 1, 2018

ryelle May 1, 2018

ryelle May 1, 2018

justinshreve commented May 2, 2018

blowery commented May 2, 2018

blowery May 2, 2018

justinshreve May 2, 2018

blowery commented May 2, 2018

jsnajdr commented May 2, 2018

blowery commented May 2, 2018

ryelle commented May 2, 2018

blowery left a comment

Store: Fix ESLint errors in WCS, tests, lib, and components directories #24571

Store: Fix ESLint errors in WCS, tests, lib, and components directories #24571

Conversation

justinshreve commented Apr 30, 2018

matticbot commented Apr 30, 2018

ryelle left a comment

Choose a reason for hiding this comment

ryelle May 1, 2018

Choose a reason for hiding this comment

ryelle May 1, 2018

Choose a reason for hiding this comment

ryelle May 1, 2018

Choose a reason for hiding this comment

justinshreve commented May 2, 2018

blowery commented May 2, 2018

blowery May 2, 2018

Choose a reason for hiding this comment

justinshreve May 2, 2018

Choose a reason for hiding this comment

blowery commented May 2, 2018

jsnajdr commented May 2, 2018

blowery commented May 2, 2018

ryelle commented May 2, 2018

blowery left a comment

Choose a reason for hiding this comment