Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependency updates #2326

Merged
merged 3 commits into from
Oct 18, 2019
Merged

Dependency updates #2326

merged 3 commits into from
Oct 18, 2019

Conversation

BPScott
Copy link
Member

@BPScott BPScott commented Oct 18, 2019

WHY are these changes introduced?

Keeping up with latest, and unblocking storybook updates, as our old version of sewing-kit was preventing storybook updates

WHAT is this pull request doing?

Updates sewing-kit, updates storybook.
Code updates to pass linting as this included a new version of eslint-plugin-shopify

Linting fixes are mostly:

How to 🎩

Ensure lint, tests and type check all pass

Fix linting issues raised by update to new version of
eslint-plugin-shopify
@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2019

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified29
Files potentially affected147

Details

All files potentially affected (total: 147)
📄 .eslintrc (total: 0)

Files potentially affected (total: 0)

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

📄 package.json (total: 0)

Files potentially affected (total: 0)

📄 scripts/new-version-pr-generator.js (total: 0)

Files potentially affected (total: 0)

📄 scripts/utilities/retry.js (total: 0)

Files potentially affected (total: 0)

🧩 sewing-kit.config.ts (total: 0)

Files potentially affected (total: 0)

🧩 src/components/AppProvider/tests/AppProvider.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Autocomplete/tests/Autocomplete.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Banner/tests/Banner.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/DropZone/tests/DropZone.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/DropZone/utils/index.ts (total: 1)

Files potentially affected (total: 1)

🧩 src/components/MediaQueryProvider/tests/MediaQueryProvider.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Navigation/tests/Navigation.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/OptionList/OptionList.tsx (total: 2)

Files potentially affected (total: 2)

🧩 src/components/PageActions/tests/PageActions.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/PolarisTestProvider/tests/PolarisTestProvider.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ResourceList/ResourceList.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ResourceList/components/BulkActions/tests/BulkActions.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ScrollLock/tests/ScrollLock.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Scrollable/tests/Scrollable.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ThemeProvider/tests/ThemeProvider.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/components/UnstyledLink/UnstyledLink.tsx (total: 71)

Files potentially affected (total: 71)

🧩 src/utilities/color-transformers.ts (total: 143)

Files potentially affected (total: 143)

🧩 src/utilities/i18n/I18n.ts (total: 104)

Files potentially affected (total: 104)

🧩 src/utilities/merge.ts (total: 105)

Files potentially affected (total: 105)

🧩 src/utilities/pick.ts (total: 6)

Files potentially affected (total: 6)

🧩 src/utilities/tests/use-is-after-initial-mount.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/utilities/unique-id/tests/hooks.test.tsx (total: 0)

Files potentially affected (total: 0)

📄 yarn.lock (total: 0)

Files potentially affected (total: 0)


This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

@BPScott BPScott requested a review from amrocha October 18, 2019 19:17
@@ -19,9 +19,6 @@ export default function sewingKitConfig(
plugins.jest((config: InitialOptions) => {
config.roots = [join(__dirname, 'src'), join(__dirname, 'tests')];

config.setupFiles.push(join(tests, 'setup.ts'));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sewing-kit now automatically adds these files to the setupFiles and setupFilesAfterEnv blocks if they exist, so we don't need to be explict here.

scripts/utilities/retry.js Outdated Show resolved Hide resolved
@@ -7,9 +7,10 @@ import {useLink, LinkLikeComponentProps} from '../../utilities/link';
// but the props explorer isn't smart enough to work that out
export interface UnstyledLinkProps extends LinkLikeComponentProps {}

// This does have a display name, but the linting has a bug in it
// https://github.com/yannickcr/eslint-plugin-react/issues/2324
// eslint-disable-next-line react/display-name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, still needed. We're using eslint-plugin-eslint-comments which contains rules than trigger a warning when we've got a disable line that doesn't do anything, so if this was unneeded it would be flagged.

This issue has been resolved, and is in v7.15.0 but eslint-plugin-shopify depends up on v7.14.3 exactly.

On eslint-plugin-shopify's master branch that dependency has been bumped, so the next time theres a release of eslint-plugin-shopify this can be fixed up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Found it a bit strange that the eslint-disable-next-line was moved.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a version bumb to eslint-plugin-react but not to the version we need - Now it raises a warning on both the React.memo and React.forwardRef - both of which are spurious.

Tbh there's no value in wrapping this in a memo anyway so I'm planning to remove that in a later PR.

@BPScott BPScott requested a review from kaelig October 18, 2019 20:52
@BPScott BPScott merged commit 9bccf6c into master Oct 18, 2019
@BPScott BPScott deleted the dep-update branch October 18, 2019 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants