Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Drop node 12 support, remove wildcard exports, update browser support #2277

Merged
merged 9 commits into from
May 19, 2022

Conversation

vsumner
Copy link
Collaborator

@vsumner vsumner commented May 18, 2022

Description

Node 12 is not longer supported.

This PR drops support to unblock updating dependencies that no longer support 12.
It also includes the following breaking changes

  • remove wildcard export
  • update browser support

I also removed the node 12 tests.

Fixes (issue #)

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

@BPScott
Copy link
Member

BPScott commented May 18, 2022

For node versions, as we're making breaking changes we should ensure we limit to versions that fully support the exports field and generally "when esmodules was marked as stable" - this was in v14.17.0, v16.0.0. v15 doesn't consider modules as stable and it is EOL so I don't think we should bother supporting it.

The engines field range should be ^14.17.0 || >=16.0.0. (you can see this syntax in use in loom's packages). We can get more aggressive to only support from when long-term-support was activated (i.e. ^14.17.0 || >=16.13.0) if desired, but I don't think that's needed.

We should also take this opportunity as chance to remove the "./*": "./*" export mapping and finally proclaim "The only allowed imports are the ones specified in the exports list"

While we're making breaking changes to version support, we should also bump the browser support versions per #1970. It'd be nice to reevaluate the polyfills stuff t-o but I think we can punt the polyfills to a later major version.

@BPScott
Copy link
Member

BPScott commented May 18, 2022

After this gets merged, I'd want to merge #2231 before cutting the new major versions - that faker update is technically a breaking change as we reexport faker.


Rolling this out in web is going to require a bit of work, as transitive dependencies shall mean duplication. What we can do is:

  • update web to use the new major versions, adding resolutions to force the new major version is used by transitive dependencies.
  • Make PRs to update all the transitive dependencies that rely on older versions of the quilt packages, and badger those teams to get them into web (e.g. online-store-ui depends upon @shopify/react-hooks and @shopify/css-utilities).
  • As updated dependencies make their way into web we remove the resolutions.

@vsumner vsumner changed the title [WIP] Drop node 12 support [WIP] Drop node 12 support, remove wildcard exports, update browser support May 18, 2022
@vsumner vsumner changed the title [WIP] Drop node 12 support, remove wildcard exports, update browser support Drop node 12 support, remove wildcard exports, update browser support May 18, 2022
@vsumner vsumner marked this pull request as ready for review May 18, 2022 20:55
@vsumner vsumner requested a review from a team as a code owner May 18, 2022 20:55
@vsumner vsumner requested a review from znja May 18, 2022 20:55
@caution-tape-bot
Copy link

👋 It looks like you're updating JavaScript packages that are known
to cause problems when duplicated.

You can deduplicate them with the yarn-deduplicate utility:

npx yarn-deduplicate --packages graphql react react-dom webpack
npx yarn-deduplicate --scopes @shopify @apollo

If running these commands doesn't produce a change in your yarn.lock file,
you didn't have duplications in these packages and can carry on.

A duplicate React version may cause an invalid hook call warning.

React context providers usually use module-scoped globals as their
default context values. Multiple versions of such packages will yield
multiple global instances, resulting in oblique runtime errors.

@BPScott BPScott self-requested a review May 18, 2022 20:59
@@ -17,7 +17,7 @@ jobs:
# output to 14.x and 16.x, so for now lock to the last version in 14
# and 16 before the ICU change.
# Once we drop support for node 12 we can consider using 14.x and 16.x
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Once we drop support for node 12 we can consider using 14.x and 16.x

Copy link
Contributor

Choose a reason for hiding this comment

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

Or too soon?

Copy link
Collaborator Author

@vsumner vsumner May 18, 2022

Choose a reason for hiding this comment

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

I didn't want to add too much overhead to this PR with too many changes. I thought it could be a follow up.

Copy link
Member

Choose a reason for hiding this comment

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

This is going to result in needing to update some expected test output, so +1 to a separate PR

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.

Ran a build and compared it against master and confirmed that we're now seeing a wild reduction in babel helpers required - no more needing to transform object spread or async/await syntaxes is huge 🎉

find packages -name _rollupPluginBabelHelpers.mjs now shows only 3 packages that need syntax transforms!

I dropped a comment inline about updating the text of the changelog entry. Update that, and update shipit/production.yml to remove the reference to node-tests (12.22) and then this is good to go.


Before this is released as a new major version, I'd want to rebase and merge #2231, and update the ci node versions so they go back to 14.x and 16.x, and use 16.14 locally. (see #2171 for context)


### Breaking Change

- Drop support for node 12. Remove wildcard export. Update `@shopify/browserslist-config`. [[#2277](https://github.com/Shopify/quilt/pull/2277)]
Copy link
Member

Choose a reason for hiding this comment

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

Consumers don't care about the change to @shopify/browserslist-config, they care about the impact that it has, What do you think to using this as the changelog line?

Suggested change
- Drop support for node 12. Remove wildcard export. Update `@shopify/browserslist-config`. [[#2277](https://github.com/Shopify/quilt/pull/2277)]
- Drop support for node 12 and Safari 10, 11 and 12. Remove wildcard export in exports field. [[#2277](https://github.com/Shopify/quilt/pull/2277)]

@@ -17,7 +17,7 @@ jobs:
# output to 14.x and 16.x, so for now lock to the last version in 14
# and 16 before the ICU change.
# Once we drop support for node 12 we can consider using 14.x and 16.x
Copy link
Member

Choose a reason for hiding this comment

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

This is going to result in needing to update some expected test output, so +1 to a separate PR

@BPScott
Copy link
Member

BPScott commented May 18, 2022

Oh! you'll need to update shipit.production.yml to remove the reference to node-tests (12.22)

@vsumner
Copy link
Collaborator Author

vsumner commented May 18, 2022

Oh! you'll need to update shipit.production.yml to remove the reference to node-tests (12.22)

I did already.

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.

🎉

@vsumner vsumner merged commit 9fccd75 into main May 19, 2022
@vsumner vsumner deleted the drop-node-12 branch May 19, 2022 13:14
@shopify-shipit shopify-shipit bot temporarily deployed to production-js May 19, 2022 18:00 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js May 25, 2022 20:52 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to production-gem June 21, 2022 14:45 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants