Skip to content

Conversation

BPScott
Copy link
Member

@BPScott BPScott commented Apr 6, 2020

WHY are these changes introduced?

Partial fix for #2896

Web needed some wrangling because we try to import empty files in esnext - this is because those files contained types but they all go stripped away during the build. Let's make sure we don't try to reference empty files so consuming apps complain about us doing so

WHAT is this pull request doing?

  • Don't use export * for files that contain only types, as this may cause issues where you try and export * from a types-only file and that will result in the export not being elided

That's just a band aid! what's the real fix?

We want to get this change out ASAP so we unblock other consumers from updating.

The long-term fix is to urn on importsNotUsedAsValues: error in tsconfig.json, which will force us to use import type/export type when referring to types, which makes these import get elided.

How to 🎩

Cut a beta release, and try and use it in web while deleting the sewing-kit.config.ts changes added in https://github.com/Shopify/web/pull/25448/files

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2020

🔴 This pull request modifies 10 files and might impact 97 other files. Because this is a larger than average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 97)
📄 README.md (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)

📄 src/components/README.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/ResourceList/components/FilterControl/index.ts (total: 1)

Files potentially affected (total: 1)

🧩 src/components/ResourceList/index.ts (total: 0)

Files potentially affected (total: 0)

🧩 src/utilities/features/index.ts (total: 94)

Files potentially affected (total: 94)

🧩 src/utilities/frame/index.ts (total: 8)

Files potentially affected (total: 8)

🧩 src/utilities/link/index.ts (total: 72)

Files potentially affected (total: 72)

🧩 src/utilities/resource-list/index.ts (total: 6)

Files potentially affected (total: 6)

@BPScott BPScott temporarily deployed to production-test April 6, 2020 20:44 Inactive
@BPScott BPScott requested a deployment to production-test April 6, 2020 20:44 Abandoned
@BPScott BPScott temporarily deployed to production-test April 6, 2020 20:44 Inactive
@BPScott BPScott requested a review from alex-page April 6, 2020 22:58
Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

Looks good, tophatted web and builds pass 💯

@BPScott BPScott merged commit 9b00837 into master Apr 6, 2020
@BPScott BPScott deleted the export-type-fiddling branch April 6, 2020 23:12
athornburg pushed a commit to athornburg/polaris-react that referenced this pull request Apr 15, 2020
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