-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
chore: reexport themes separately #13268
Conversation
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Potential regressions comparing to master
Perf comparison
Perf tests with no regressions
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 77ade9a313abc59753fb7550cd050ffc38678d44 (build) |
// | ||
export { themes }; | ||
export { default as teamsTheme } from './themes/teams'; |
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.
This would be a breaking change for downstream consumers importing themes
currently. Make sure to add a CHANGELOG note.
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.
Add breaking changelog note :)
* chore: reexport themes separately * add changelog entry
Right now, if you try and copy the code from the Quickstart guide, you'll end up with an error about "teamsTheme" not being defined. This is because we're importing "themes" instead of "teamsTheme". I suspect this was just glanced over in this PR: microsoft#13268
Pull request checklist
$ yarn change
BREAKING CHANGES
This PR removes
themes
export from@fluentui/react-northstar
. Instead of it please use direct exports of themes.Before
After
Description of changes
fluentui/packages/fluentui/react-northstar/src/index.ts
Lines 1 to 6 in 57c30dd
Existing reexport breaks treeshaking because it fails
ModuleConcatenation
process in Webpack:If Webpack fails to concatenate modules it makes dead-code elimination via Terser almost useless 💣
Measures are taken from
packages/fluentui/docs/src/examples/components/Accordion/Performance/AccordionDefault.bsize.tsx
with enabledconcatenateModules
inwebpack.config.stats.ts
.Before
873KB
After
704KB (-169KB/20%)
As a side-effect it also decouples themes. As namespace reexports are handled by Webpack in a different way than in Rollup they were all included to bundles.