-
Notifications
You must be signed in to change notification settings - Fork 86
build: remove babel, configure swc to be main test transpiler #7743
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
base: master
Are you sure you want to change the base?
Conversation
This removes all unused exports within the codebase which were not being directly imported or tested elsewhere. It also removes superfluous exports where a const is exported as both a named or default export If any exports cannot be removed because they are used in other files which are not directly tested, new tests have been added BREAKING CHANGE: If any consumers are importing any of these exports which have been removed this will break their application. In theory no consumers should be importing implementation details into their project, however we have marked this change as breaking as there is a very remote possibility that some projects could break due to this change
| import compareBuild from "semver/functions/compare-build"; | ||
|
|
||
| import { TOOL_ID } from "./constants"; | ||
| import { TOOL_ID } from "./constants/constants"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: you can just make constants.ts an index.ts
| return ( | ||
| <div ref={trapRef}> | ||
| {isOpen && ( | ||
| <div |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment: not part of your changes but we should add the following to the top of the file imo so we don't get the linter warnings anymore
/* eslint-disable jsx-a11y/no-noninteractive-tabindex */
/* eslint-disable jsx-a11y/no-static-element-interactions */
| "<rootDir>/lib", | ||
| "<rootDir>/esm", | ||
| ".*index\\.ts$", | ||
| ".*[.-]config\\.ts$", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth generating a report from a test run in master and comparing it to one from this PR
| } | ||
|
|
||
| export const ActionPopoverItem = ({ | ||
| const ActionPopoverItem = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.


Proposed behaviour
Removes all
babelrelated dependencies and instead usesswc/jestas the main test transpiler. In order to transition toswc/jestsome additional changes have also been made.babel/jest's less aggressive tree-shaking meant it would often leave unused exports in place during transpilation.swc/jesttakes a stricter approach - it analyses exports more carefully and expects us to be more explicit about what we're exporting and using.All exports which are unused (not imported anywhere else internally) were highlighted and have had to be removed.
There were multiple instances of consts which were only used locally in one file being exported, or instances of some costs being exported and never being used locally or elsewhere.
In some files we have also "doubled up" our exports, exporting twice as a named export and a default export. Where possible the named export has been removed, and only the default export remains.
Some additional steps have been made to meet coverage requirements with some exports which are needed but not directly tested, as
instanbul ignoreis now ineffectual against uncovered const statementsThe removal of unused exports has been marked as a breaking change as technically some consumers may have imported these directly from legacy projects etc. However, the actual likelihood that this will break consumers' applications is slim to none.
config.tsandindex.tsfiles are now exempt from the coverage report, asconfig.tsconsts are typically used in stories and therefore are treated as unused.index.tsfiles exports are consumer facing entry points, so they have also been exempt as some exports are never imported and used elsewhere.Current behaviour
Currently, there are multiple
babeldependencies which are unused andbabel/jestis used for test transpilation.Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions