fix(unplugin): arithmetic support for defineConsts#1707
Conversation
|
@skovhus is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
|
@nmn if we don't want to support arithmetic for some reason (would be nice if we could), then at least I think we should fail hard instead of silently emitting broken CSS. |
|
We just had another production issue due to the underlying issue. @nmn @mellyeliu would love some feedback on the approach on this branch. Happy to put up a PR that fails hard if we don't want arithmetic. |
|
Thanks for fixing this, overall this implementation looks fine and keeps everything per-file. Mind adding some more tests for different arithmetic operations (a + b * c, (a + b) * 2, with brackets, etc) plus a couple of string-concatenation cases like 'translateX(' + token + ')'? |
|
Another foot gun that I'll need to see if we can solve here: // $zIndex.dialog is defined as a number
const PRESENTER_Z_INDEX = Number($zIndex.dialog) + 1;
const styles = stylex.create({
presenter: {
zIndex: PRESENTER_Z_INDEX, // silently breaks
},
}); |
- Allow ==/!=/===/!== null and undefined guards on token refs (previously-correct
defensive patterns keep compiling)
- Add comparison-specific compile error message for other comparisons
- Fail on jammed '+' concatenation (token + '10px') instead of emitting
invalid CSS; separated list concatenation ('solid ' + token) still works
- Reject calc() expressions in computed style keys
- Throw token-misuse errors inside dynamic styles instead of silently
degrading to runtime inline styles
- Preserve evaluator deopt reasons through defineVars function values
- Widen var() detection to cover unicode custom-property names
- Validate calc dimension operands via postcss-value-parser + CSS unit
whitelist (rejects bogus units like '10foo')
- Share evaluationError helper across all evaluating visitors (keyframes,
positionTry, viewTransitionClass, nested variants included)
- Extract shared roundForCss from transform-value
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c50df94 to
dcf8d05
Compare
What changed / motivation ?
JS arithmetic on
defineVars/defineConststoken references insidestylex.create()silently produced broken CSS (C.a + C.b→"var(--a)var(--b)",C.a - 1→NaN).This PR compiles arithmetic to CSS
calc()instead — without violating the "transforming a file must not read other files" constraint: the emitted expression is built from the same hash-derivedvar()strings as today, anddefineConstsvalues are substituted by the existing CSS aggregation step.zIndex: C.a + C.b→calc(var(--a) + var(--b))(→calc(26 + 14)for consts)zIndex: C.elevated - 1→calc(var(--elevated) - 1)marginLeft: -C.gutter→calc(-1 * var(--gutter))defineConsts,defineVars(incl. sibling refs), and mixed expressionsEverything that can't map to
calc()is now a compile-time error.Breaking change
C.a === 'red' ? x : y) previously evaluated silently (string comparison, usually picking the wrong branch) and now fail the build with an actionable error. Null/undefined guards are exempt and keep working.Linked PR/Issues
Fixes #1597
Additional Context
Screenshots, Tests, Anything Else
Pre-flight checklist
Contribution Guidelines