[babel-plugin] Fix theming in dev/debug mode #1101
Conversation
packages/@stylexjs/babel-plugin/src/shared/stylex-vars-utils.js
Outdated
Show resolved
Hide resolved
workflow: benchmarks/perfComparison of performance test results, measured in operations per second. Larger is better.
|
workflow: benchmarks/sizeComparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.
|
e43e763 to
0ea39b3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
0ea39b3 to
15530d1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Although this seems to fix the "reactive variables" idea that many have, it has a major regression in how themes can be used and makes it impossible to have themes for separate varGroups managed separately.
"reactive variables" would also not quite work how people expect as it would only work on one comment at a time.
packages/@stylexjs/babel-plugin/__tests__/transform-import-export-test.js
Outdated
Show resolved
Hide resolved
packages/@stylexjs/babel-plugin/__tests__/transform-process-test.js
Outdated
Show resolved
Hide resolved
15530d1 to
1a48787
Compare
defineVars1a48787 to
84bd3bb
Compare
| { | ||
| [themeVars.__themeName__]: themeClass, | ||
| $$css: true, | ||
| }, |
There was a problem hiding this comment.
This change avoids the need for the flowfixme comment
| const varSafeKey = | ||
| key === '__themeName__' | ||
| ? '' | ||
| : (key[0] >= '0' && key[0] <= '9' ? `_${key}` : key).replace( | ||
| /[^a-zA-Z0-9]/g, | ||
| '_', | ||
| ) + '-'; |
There was a problem hiding this comment.
This is a fix for dev/debug mode. The way theming is currently designed to work is broken in dev because this generates bad class names that don't map to anything in the generated CSS
|
Dev/debug changes lgtm |
This fixes a bug in StyleX theming. Before, defineVars was creating a `:root` rule with a classname that is unique to a given defineVars call. However, when debug mode is enabled, calls to createTheme were generating bad debug class names and the unique classname for vars was not being applied. This bug isn't caught by the unit tests because they are using fixtures and not using the bad logic in evaluate-path.js
84bd3bb to
c5490c8
Compare
This fixes a bug in StyleX theming caused by how evaluate-path works in dev/debug mode, which produced bad debug class names for
createThemecalls.