Skip to content

Conversation

@BPScott
Copy link
Member

@BPScott BPScott commented Apr 7, 2020

WHY are these changes introduced?

Follow up to #2897
Fixes #2896

By ensuring we mark type-only imports we make sure that we avoid
deoptimisations and export issues if compiled files reference empty
files (which are empty because types have no compile time output)

WHAT is this pull request doing?

Sets importsNotUsedAsValues to error in our tsconfig and change any import {SomeType} references to import type {SomeType} that it flags.

How to 🎩

Ensure that type-check passes.
Ensure that the build output does not change, by doing the following:

  • Checkout the parent of this PR, do a build and copy the esnext folder
    • git checkout 27842a6389d2ebd073463fa5f8f2d47dcdc3a035 && yarn build && mv esnext esnext-base
  • Checkout this branch, do a build and compare the contents of the esnext folder to the copy you just created
    • git checkout ts-importsNotUsedAsValues && yarn build && diff -ru esnext-base esnext
  • Note that the build output is identical

@BPScott BPScott force-pushed the ts-importsNotUsedAsValues branch from c198568 to e357175 Compare April 7, 2020 01:19
@BPScott BPScott requested a review from alex-page April 7, 2020 01:19
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2020

🟡 This pull request modifies 30 files and might impact 61 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 61)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 scripts/analyze-custom-properties/tests/fixtures.ts (total: 0)

Files potentially affected (total: 0)

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

Files potentially affected (total: 0)

🧩 src/components/ActionList/ActionList.tsx (total: 54)

Files potentially affected (total: 54)

🧩 src/components/ActionList/components/Item/Item.tsx (total: 56)

Files potentially affected (total: 56)

🧩 src/components/ActionList/components/Section/Section.tsx (total: 55)

Files potentially affected (total: 55)

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

Files potentially affected (total: 2)

🧩 src/components/ActionMenu/components/MenuAction/MenuAction.tsx (total: 4)

Files potentially affected (total: 4)

🧩 src/components/ActionMenu/components/MenuGroup/MenuGroup.tsx (total: 3)

Files potentially affected (total: 3)

🧩 src/components/ActionMenu/components/RollupActions/RollupActions.tsx (total: 3)

Files potentially affected (total: 3)

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

Files potentially affected (total: 0)

🧩 src/components/ActionMenu/utilities.ts (total: 3)

Files potentially affected (total: 3)

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

Files potentially affected (total: 0)

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

Files potentially affected (total: 0)

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

Files potentially affected (total: 0)

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

Files potentially affected (total: 2)

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

Files potentially affected (total: 0)

🧩 src/components/Button/Button.tsx (total: 52)

Files potentially affected (total: 52)

🧩 src/components/Button/utils.tsx (total: 51)

Files potentially affected (total: 51)

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

Files potentially affected (total: 0)

🧩 src/components/Card/Card.tsx (total: 3)

Files potentially affected (total: 3)

🧩 src/components/Card/components/Header/Header.tsx (total: 4)

Files potentially affected (total: 4)

🧩 src/components/Card/components/Section/Section.tsx (total: 4)

Files potentially affected (total: 4)

🧩 src/components/Choice/Choice.tsx (total: 7)

Files potentially affected (total: 7)

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

Files potentially affected (total: 0)

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

Files potentially affected (total: 0)

🧩 src/components/ColorPicker/components/AlphaPicker/AlphaPicker.tsx (total: 1)

Files potentially affected (total: 1)

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

Files potentially affected (total: 0)

🧩 src/components/DataTable/components/Cell/Cell.tsx (total: 1)

Files potentially affected (total: 1)

🧩 src/components/DataTable/components/Navigation/Navigation.tsx (total: 1)

Files potentially affected (total: 1)

By ensuring we mark type-only imports we make sure that we avoid
deoptimisations and export issues if compiled files reference empty
files (which are empty because types have no compile time output)
@BPScott BPScott force-pushed the ts-importsNotUsedAsValues branch from e357175 to 6f6df1b Compare April 8, 2020 00:09
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 @BPScott 💯

@BPScott BPScott merged commit 69ec09d into master Apr 9, 2020
@BPScott BPScott deleted the ts-importsNotUsedAsValues branch April 9, 2020 18:10
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.

esnext builds are deoptimized by empty types.js files

2 participants