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

Update developer dependencies #1327

Merged
merged 2 commits into from
May 1, 2019
Merged

Update developer dependencies #1327

merged 2 commits into from
May 1, 2019

Conversation

AndrewMusgrave
Copy link
Member

WHY are these changes introduced?

Second attempt to update deps 😄 #1187

@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 16, 2019 20:56 Inactive
@AndrewMusgrave AndrewMusgrave mentioned this pull request Apr 16, 2019
@AndrewMusgrave AndrewMusgrave self-assigned this Apr 16, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 17, 2019 20:26 Inactive
@BPScott
Copy link
Member

BPScott commented Apr 23, 2019

Looks like this adds a lot of duplicates, can we remove some by running an npx yarn-deduplicate yarn.lock?

It looks like we'd also want to pull in the latest version of babel-plugin-shopify to shut up the coreJS complaining like what SK does in https://github.com/Shopify/sewing-kit/pull/1217

@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 24, 2019 16:59 Inactive
@AndrewMusgrave AndrewMusgrave requested a review from BPScott April 24, 2019 17:00
@AndrewMusgrave AndrewMusgrave changed the title Update dependencies [WIP] Update dependencies Apr 24, 2019
@BPScott
Copy link
Member

BPScott commented Apr 24, 2019

In my poking around with #1344 I think I've learnt that we want to either: update @babel/core and babel-plugin-shopify, or leave them both where they were.

Using the latest will mean that the stand-alone build will be a bit bigger due to including corejs polyfills thanks to the babel-preset-env now using useBuiltIns: 'usage' (which changed in babel-plugin-shopify v18). This is due to our support for old ios. This doesn't affect polaris when used as part of sewing-kit because sk doesn't use the stand-alone build.

I don't mind which one you go for.

@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 29, 2019 13:47 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 29, 2019 18:44 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 29, 2019 18:54 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1327 April 29, 2019 19:02 Inactive
@AndrewMusgrave AndrewMusgrave changed the title [WIP] Update dependencies Update dependencies Apr 29, 2019
@AndrewMusgrave
Copy link
Member Author

Ending up keeping this scoped to devDependencies, and deduping causes broken builds. Build seems to be passing now with all checks ✅

Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Not tried this locally but tests are green and storybook looks good :shipit:

@alex-page alex-page changed the title Update dependencies Update developer dependencies Apr 30, 2019
Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

Doh, that last review should have been green :shipit:

@AndrewMusgrave AndrewMusgrave merged commit 7f2b5aa into master May 1, 2019
@AndrewMusgrave AndrewMusgrave deleted the update-deps-take-2 branch May 1, 2019 16:43
@chloerice chloerice temporarily deployed to production May 8, 2019 19:57 Inactive
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.

3 participants