feat: better dev theme names #803
Conversation
facebook-github-bot
added
the
CLA Signed
workflow: benchmarks/sizeComparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.
|
| } = objMap(variables, (value, key) => { | ||
| // Created hashed variable names with fileName//themeName//key | ||
|
|
||
| const processedKey = key.startsWith('--') ? key.slice(2) : key; | ||
| const themeNamePrefix = debug ? `${processedKey}` : ''; |
There was a problem hiding this comment.
let's sync on what we want to prefix here, I'm putting processedKey as a placeholder but can be modified to be themeName-processedKey etc. I noticed in the testing that some of the themeNames include special characters beyond alphanumeric and dash symbols, which could potentially break things, so it'd be good to validate
There was a problem hiding this comment.
If the key starts with -- don't add a prefix at all. In those cases we don't generate a variable name and use the key name as is.
| "rtl": null, | ||
| }, | ||
| "bgColorDisabled-x568ih9-kpd015": { | ||
| "ltr": "@supports (color: oklab(0 0 0)){:root, .bgColorDisabled-x568ih9{--xpegid5:oklab(0.7 -0.3 -0.4);}}", |
There was a problem hiding this comment.
Is this output correct? It looks like the source is a "supports" nested in a "media" for dark mode, but the output is missing the media part.
It's a little difficult to read the test because there are several patterns in the input and a lot of generated content as a result. It might be worth breaking it up so it's easier to check the output by eye
There was a problem hiding this comment.
Yes, --xpegid5: doesn't have the prefix.
Instead the className that defines the variables has the prefix which is likely wrong.
|
updated to prefix the variable names with key as discussed with @nmn |
packages/babel-plugin/__tests__/stylex-transform-define-vars-test.js
Outdated
Show resolved
Hide resolved
packages/shared/__tests__/define-vars/stylex-define-vars-test.js
Outdated
Show resolved
Hide resolved
packages/babel-plugin/__tests__/stylex-transform-define-vars-test.js
Outdated
Show resolved
Hide resolved
* prefix dev classnames with property name * added debug config and babel tests * add caching to default path
* prefix dev classnames with property name * added debug config and babel tests * add caching to default path
What changed / motivation ?
Let's modify the themeNameHash in debug mode to include the key before the hash like so:
Tests
Added a basic test to cover theme names when debug is on
Pre-flight checklist
Contribution Guidelines