feat(react-theme-sass): Add SASS variables mapped to CSS variables provided by react-theme#20808
feat(react-theme-sass): Add SASS variables mapped to CSS variables provided by react-theme#20808miroslavstastny wants to merge 9 commits intomicrosoft:masterfrom
Conversation
…ovided by react-theme
📊 Bundle size reportUnchanged fixtures
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 84c2858:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: d525c8b2106551865d648873210a6b48c2326ecb (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| ContextualMenu | mount | 15514 | 7253 | 1000 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 832 | 825 | 5000 | |
| BaseButton | mount | 794 | 814 | 5000 | |
| Breadcrumb | mount | 2309 | 2303 | 1000 | |
| ButtonNext | mount | 442 | 437 | 5000 | |
| Checkbox | mount | 1343 | 1328 | 5000 | |
| CheckboxBase | mount | 1122 | 1137 | 5000 | |
| ChoiceGroup | mount | 4108 | 4121 | 5000 | |
| ComboBox | mount | 855 | 863 | 1000 | |
| CommandBar | mount | 8861 | 8824 | 1000 | |
| ContextualMenu | mount | 15514 | 7253 | 1000 | Possible regression |
| DefaultButton | mount | 1000 | 996 | 5000 | |
| DetailsRow | mount | 3303 | 3252 | 5000 | |
| DetailsRowFast | mount | 3280 | 3241 | 5000 | |
| DetailsRowNoStyles | mount | 3099 | 3112 | 5000 | |
| Dialog | mount | 2232 | 2231 | 1000 | |
| DocumentCardTitle | mount | 155 | 156 | 1000 | |
| Dropdown | mount | 2760 | 2772 | 5000 | |
| FluentProviderNext | mount | 1661 | 1784 | 5000 | |
| FluentProviderWithTheme | mount | 140 | 126 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 92 | 100 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 175 | 188 | 10 | |
| FocusTrapZone | mount | 1597 | 1642 | 5000 | |
| FocusZone | mount | 1566 | 1546 | 5000 | |
| IconButton | mount | 1520 | 1517 | 5000 | |
| Label | mount | 329 | 316 | 5000 | |
| Layer | mount | 2594 | 2575 | 5000 | |
| Link | mount | 430 | 433 | 5000 | |
| MakeStyles | mount | 1477 | 1455 | 50000 | |
| MenuButton | mount | 1272 | 1265 | 5000 | |
| MessageBar | mount | 1720 | 1745 | 5000 | |
| Nav | mount | 2874 | 2858 | 1000 | |
| OverflowSet | mount | 955 | 964 | 5000 | |
| Panel | mount | 2152 | 2168 | 1000 | |
| Persona | mount | 732 | 756 | 1000 | |
| Pivot | mount | 1237 | 1271 | 1000 | |
| PrimaryButton | mount | 1112 | 1142 | 5000 | |
| Rating | mount | 6645 | 6558 | 5000 | |
| SearchBox | mount | 1150 | 1149 | 5000 | |
| Shimmer | mount | 2182 | 2159 | 5000 | |
| Slider | mount | 1693 | 1661 | 5000 | |
| SpinButton | mount | 4295 | 4277 | 5000 | |
| Spinner | mount | 394 | 398 | 5000 | |
| SplitButton | mount | 2721 | 2690 | 5000 | |
| Stack | mount | 472 | 466 | 5000 | |
| StackWithIntrinsicChildren | mount | 1992 | 1995 | 5000 | |
| StackWithTextChildren | mount | 4467 | 4503 | 5000 | |
| SwatchColorPicker | mount | 10604 | 9728 | 5000 | |
| TagPicker | mount | 2338 | 2312 | 5000 | |
| TeachingBubble | mount | 11320 | 11449 | 5000 | |
| Text | mount | 390 | 393 | 5000 | |
| TextField | mount | 1212 | 1183 | 5000 | |
| ThemeProvider | mount | 1050 | 1044 | 5000 | |
| ThemeProvider | virtual-rerender | 558 | 549 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1633 | 1607 | 5000 | |
| Toggle | mount | 700 | 701 | 5000 | |
| buttonNative | mount | 138 | 131 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| PortalMinimalPerf.default | 166 | 154 | 1.08:1 |
| HeaderMinimalPerf.default | 327 | 307 | 1.07:1 |
| AvatarMinimalPerf.default | 177 | 169 | 1.05:1 |
| ListNestedPerf.default | 491 | 467 | 1.05:1 |
| SkeletonMinimalPerf.default | 305 | 291 | 1.05:1 |
| ButtonMinimalPerf.default | 148 | 142 | 1.04:1 |
| ChatWithPopoverPerf.default | 338 | 324 | 1.04:1 |
| GridMinimalPerf.default | 296 | 285 | 1.04:1 |
| ListCommonPerf.default | 547 | 528 | 1.04:1 |
| LoaderMinimalPerf.default | 596 | 575 | 1.04:1 |
| BoxMinimalPerf.default | 298 | 289 | 1.03:1 |
| CarouselMinimalPerf.default | 413 | 401 | 1.03:1 |
| ListWith60ListItems.default | 559 | 544 | 1.03:1 |
| AlertMinimalPerf.default | 233 | 229 | 1.02:1 |
| ButtonSlotsPerf.default | 475 | 465 | 1.02:1 |
| CardMinimalPerf.default | 482 | 474 | 1.02:1 |
| DividerMinimalPerf.default | 314 | 308 | 1.02:1 |
| FlexMinimalPerf.default | 251 | 246 | 1.02:1 |
| HeaderSlotsPerf.default | 655 | 640 | 1.02:1 |
| ImageMinimalPerf.default | 325 | 320 | 1.02:1 |
| InputMinimalPerf.default | 1112 | 1090 | 1.02:1 |
| LabelMinimalPerf.default | 335 | 330 | 1.02:1 |
| LayoutMinimalPerf.default | 316 | 311 | 1.02:1 |
| ListMinimalPerf.default | 441 | 432 | 1.02:1 |
| SegmentMinimalPerf.default | 302 | 297 | 1.02:1 |
| StatusMinimalPerf.default | 577 | 568 | 1.02:1 |
| TextAreaMinimalPerf.default | 425 | 416 | 1.02:1 |
| TreeMinimalPerf.default | 674 | 664 | 1.02:1 |
| AnimationMinimalPerf.default | 467 | 463 | 1.01:1 |
| ButtonOverridesMissPerf.default | 1440 | 1424 | 1.01:1 |
| ChatMinimalPerf.default | 636 | 627 | 1.01:1 |
| CheckboxMinimalPerf.default | 2284 | 2251 | 1.01:1 |
| DatepickerMinimalPerf.default | 4709 | 4669 | 1.01:1 |
| FormMinimalPerf.default | 353 | 348 | 1.01:1 |
| ItemLayoutMinimalPerf.default | 1017 | 1011 | 1.01:1 |
| ProviderMergeThemesPerf.default | 1481 | 1472 | 1.01:1 |
| ProviderMinimalPerf.default | 967 | 958 | 1.01:1 |
| RadioGroupMinimalPerf.default | 390 | 387 | 1.01:1 |
| ToolbarMinimalPerf.default | 790 | 785 | 1.01:1 |
| DialogMinimalPerf.default | 640 | 643 | 1:1 |
| DropdownMinimalPerf.default | 2558 | 2552 | 1:1 |
| EmbedMinimalPerf.default | 3499 | 3505 | 1:1 |
| MenuMinimalPerf.default | 715 | 712 | 1:1 |
| MenuButtonMinimalPerf.default | 1413 | 1409 | 1:1 |
| ReactionMinimalPerf.default | 322 | 322 | 1:1 |
| RefMinimalPerf.default | 206 | 205 | 1:1 |
| SplitButtonMinimalPerf.default | 3653 | 3665 | 1:1 |
| TableManyItemsPerf.default | 1598 | 1603 | 1:1 |
| CustomToolbarPrototype.default | 3496 | 3491 | 1:1 |
| TooltipMinimalPerf.default | 880 | 878 | 1:1 |
| AccordionMinimalPerf.default | 133 | 135 | 0.99:1 |
| AttachmentSlotsPerf.default | 897 | 902 | 0.99:1 |
| ChatDuplicateMessagesPerf.default | 260 | 262 | 0.99:1 |
| DropdownManyItemsPerf.default | 567 | 574 | 0.99:1 |
| SliderMinimalPerf.default | 1418 | 1439 | 0.99:1 |
| IconMinimalPerf.default | 532 | 537 | 0.99:1 |
| TableMinimalPerf.default | 348 | 350 | 0.99:1 |
| TextMinimalPerf.default | 299 | 301 | 0.99:1 |
| TreeWith60ListItems.default | 152 | 154 | 0.99:1 |
| AttachmentMinimalPerf.default | 131 | 134 | 0.98:1 |
| RosterPerf.default | 982 | 1006 | 0.98:1 |
| PopupMinimalPerf.default | 520 | 528 | 0.98:1 |
| VideoMinimalPerf.default | 533 | 547 | 0.97:1 |
| @@ -0,0 +1 @@ | |||
| export {}; | |||
There was a problem hiding this comment.
Is the index.ts necessary?
There was a problem hiding this comment.
no, you should also remove the main entrypoint
| "markdown-table": "^2.0.0", | ||
| "node-plop": "^0.25.0", | ||
| "node-polyfill-webpack-plugin": "^1.0.2", | ||
| "node-sass": "^6.0.1", |
There was a problem hiding this comment.
this is now a devDependency in root package.json (without the caret).
@layershifter - is that ok?
There was a problem hiding this comment.
Not Shift :) but we've generally been trying to move dev deps to the root and remove carets, so it should be fine.
| "main": "lib/index.js", | ||
| "typings": "lib/index.d.ts", |
There was a problem hiding this comment.
These should probably be removed (along with the index file) since this package isn't for JS
| "name": "@fluentui/react-theme-sass", | ||
| "version": "9.0.0-beta.3", | ||
| "description": "SASS variables referencing react-theme design tokens injected to DOM by react-provider.", | ||
| "private": true, |
There was a problem hiding this comment.
Just to verify, is this intended to be checked in but not published yet?
| "dependencies": { | ||
| "tslib": "^2.1.0" | ||
| }, |
There was a problem hiding this comment.
Since this package doesn't ship JS (if I'm understanding right) I don't think it should depend on tslib
| "dependencies": { | |
| "tslib": "^2.1.0" | |
| }, |
| @@ -0,0 +1,29 @@ | |||
| { | |||
There was a problem hiding this comment.
At least IMO, if this package is never intended to ship JS, the API Extractor configs and output should be removed. The unfortunate part is I'm not sure whether the migration generator understands this or will just keep trying to add them back.
| "@fluentui/react-tabster": ["packages/react-tabster/src/index.ts"], | ||
| "@fluentui/react-text": ["packages/react-text/src/index.ts"], | ||
| "@fluentui/react-theme": ["packages/react-theme/src/index.ts"], | ||
| "@fluentui/react-theme-sass": ["packages/react-theme-sass/src/index.ts"], |
There was a problem hiding this comment.
There's no ts here, do we need path aliases ?
There was a problem hiding this comment.
good question, for sake of consistency I'd say yes. Also I'd keep the index.ts there with empty export / even better to throw a runtime error if someone would import that
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "plugins": ["annotate-pure-calls", "@babel/transform-react-pure-annotations"] | |||
There was a problem hiding this comment.
babel config seems redundant since there's no js code to run on
|
Be nice to get @Hotell's opinion here, since the issue of 'what to do when a package has no JS' definitely should come under the radar our repo infra plans. Also will add to our next vbuild agenda |
| "loader-utils": "2.0.0", | ||
| "memfs": "3.2.2", | ||
| "node-fetch": "2.6.7", | ||
| "node-sass": "6.0.1", |
| { | ||
| "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", | ||
| "extends": "@fluentui/scripts/api-extractor/api-extractor.common.json", | ||
| "compiler": { |
There was a problem hiding this comment.
how did you generate the scaffold ? this shouldn't be here
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "extends": ["plugin:@fluentui/eslint-plugin/node"], | |||
There was a problem hiding this comment.
hmm just to verify, this is a NODE package not BROWSER package, correct ? meaning this will be compiled away during build in consumers code.
| .map(tokenName => `$expected__${tokenName}: $${tokenName};`), | ||
| ].join('\n'); | ||
|
|
||
| renderSync({ data }); |
There was a problem hiding this comment.
what is this test testing ?
| "@fluentui/react-tabster": ["packages/react-tabster/src/index.ts"], | ||
| "@fluentui/react-text": ["packages/react-text/src/index.ts"], | ||
| "@fluentui/react-theme": ["packages/react-theme/src/index.ts"], | ||
| "@fluentui/react-theme-sass": ["packages/react-theme-sass/src/index.ts"], |
There was a problem hiding this comment.
good question, for sake of consistency I'd say yes. Also I'd keep the index.ts there with empty export / even better to throw a runtime error if someone would import that
| @@ -0,0 +1,6 @@ | |||
| $borderRadiusNone: var(--borderRadiusNone); | |||
There was a problem hiding this comment.
how did you generate these files ? I'm missing the script automation for this in the PR
|
Stale, replaced by #22964 |
Add
react-theme-sasspackage with SCSS variables mapped to v9 theme CSS variablesPull request checklist
$ yarn changeDescription of changes
There are usecases where partners want to use v9 theming in their existing SASS-based code.
The plan is to have
FluentProviderset the v9 theme and SASS styles to use the theme.The theme is accessible as a set of CSS variables managed by the provider:

But any code should never hardcode the CSS variable names as those are internal to the FUIv9 implementation and might change (e.g. we might switch to hashed variable names).
This PR adds a
react-theme-sasspackage which exports SCSS variables which are mapped to CSS variables. Should the CSS variable names change, only the mapping will change, the SCSS variable names will remain the same (those currently match the token names).Expected usage
The
@fluentui/react-theme-sasspackage will NOT be re-exported by@fluentui/react-components. It is consumer's responsibility to install the package explicitly and keep the version in sync with@fluentui/react-components.