Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

update deprecated export pattern #2184

Merged
merged 3 commits into from
Feb 25, 2022
Merged

update deprecated export pattern #2184

merged 3 commits into from
Feb 25, 2022

Conversation

dahukish
Copy link
Contributor

@dahukish dahukish commented Feb 22, 2022 •
edited
Loading

Description

Fixes (issue https://github.com/Shopify/web/issues/56743)

The ./ pattern for package exports is deprecated and ignored https://webpack.js.org/guides/package-exports/#support

Updated too ./* which is viable. Also sorted the exported order to reflect: https://webpack.js.org/guides/package-exports/#notes-about-ordering

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • (All packages) 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)

@dahukish dahukish requested a review from a team as a code owner February 22, 2022 16:57
@dahukish dahukish self-assigned this Feb 22, 2022
@heathernfran
Copy link
Contributor

heathernfran commented Feb 22, 2022 •
edited
Loading

The package.json template can also be updated

https://github.com/Shopify/quilt/blob/main/templates/package.hbs.json

@heathernfran
Copy link
Contributor

Updating this test to ./* should fix the tests

https://github.com/Shopify/quilt/blob/main/tests/consistent-package-json.test.ts#L145

@dahukish dahukish force-pushed the fix-deprecated-exports-pattern branch from 70081b6 to 8938590 Compare February 22, 2022 18:36
@shopify-shipit shopify-shipit bot temporarily deployed to fix-deprecated-exports-pattern February 22, 2022 19:45 Inactive
BPScott
BPScott reviewed Feb 23, 2022
View reviewed changes
Copy link
Member

@BPScott BPScott left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Code changes look good.

A nitpick: What's with thinking behind the reordering of the . import? It's doesn't make any difference but I'd say the what you get when you import @shopify/PACKAGENAMEHERE would be the most important export to see, and thus it deserves to be at the top of the list, above what you get when importing @shopify/PACKAGENAMEHERE/blah

packages/ast-utilities/package.json
@@ -68,12 +68,6 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
".": {
Copy link
Member

Choose a reason for hiding this comment

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

What's with thinking behind the reordering of the . import? It's doesn't make any difference but I'd say the what you get when you import @shopify/PACKAGENAMEHERE would be the most important export to see, and thus it deserves to be at the top of the list

packages/graphql-mini-transforms/package.json
@@ -95,7 +89,13 @@
"esnext": "./webpack-loader.esnext",
"import": "./webpack-loader.mjs",
"require": "./webpack-loader.js"
}
},
".": {
Copy link
Member

Choose a reason for hiding this comment

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

Same Q

packages/graphql-testing/package.json
@@ -54,16 +54,16 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
"./matchers": {
Copy link
Member

Choose a reason for hiding this comment

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

Same Q

packages/polyfills/package.json
@@ -224,11 +223,6 @@
"import": "./idle-callback.node.mjs",
"require": "./idle-callback.node.js"
},
".": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

packages/react-async/package.json
@@ -56,16 +56,16 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
"./testing": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

packages/react-html/package.json
@@ -59,16 +59,16 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
"./server": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

packages/react-network/package.json
@@ -62,16 +62,16 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
"./server": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

packages/react-server/package.json
@@ -82,16 +82,16 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
"./webpack-plugin": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

packages/react-testing/package.json
@@ -53,16 +53,16 @@
"module": "index.mjs",
"esnext": "index.esnext",
"exports": {
"./": "./",
"./matchers": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

packages/web-worker/package.json
@@ -21,12 +21,6 @@
}
},
"exports": {
".": {
Copy link
Member

Choose a reason for hiding this comment

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

One more reorder

BPScott
BPScott approved these changes Feb 25, 2022
View reviewed changes
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.

Let's get this out!

@dahukish dahukish force-pushed the fix-deprecated-exports-pattern branch from 69db61b to ce5162b Compare February 25, 2022 21:57
@dahukish dahukish merged commit 4a46cce into main Feb 25, 2022
@dahukish dahukish deleted the fix-deprecated-exports-pattern branch February 25, 2022 22:17
BPScott added a commit that referenced this pull request Feb 25, 2022
@BPScott
Add changelogs for change in #2184
821329c
@shopify-shipit shopify-shipit bot temporarily deployed to production February 25, 2022 22:56 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to mf-beta March 7, 2022 09:18 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to apollo-3-test March 15, 2022 18:42 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.
Reviewers

@BPScott BPScott BPScott approved these changes

@heathernfran heathernfran heathernfran approved these changes

@ttemrahcelik ttemrahcelik Awaiting requested review from ttemrahcelik ttemrahcelik was automatically assigned from Shopify/admin-web-platform

Assignees

@dahukish dahukish

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dahukish @heathernfran @BPScott