fix(native): a custom property with no value produces no key - #431
Open
YevheniiKotyrlo wants to merge 1 commit into
Open
fix(native): a custom property with no value produces no key#431YevheniiKotyrlo wants to merge 1 commit into
YevheniiKotyrlo wants to merge 1 commit into
Conversation
`vars()` and `<VariableContextProvider />` both mapped every entry of the record they are given straight into the runtime's variable record, so an entry whose value is `undefined` planted a key holding `undefined`. Variable lookup is presence-keyed - `varResolver` returns on `name in variables` - so that key reads as "this variable is set to nothing" and stops the cascade before the inherited value, the `:root` value and the `var()` fallback. The property it feeds is dropped rather than resolved. The compiler already holds this invariant one plane over: `pushDescriptor` returns early on an undefined value, so no compiled rule ever carries a variable entry without a value. The two runtime channels into the same record are what was missing it. Both built the entry list with the same expression, so the fix is one `toVariableRecord` beside `VAR_SYMBOL` and `VariableContextValue`, which strips the `--` prefix and skips an entry with no value. `"unset"` remains how a variable is deliberately cleared.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
vars()and<VariableContextProvider />are the two JavaScript channels into the native variable system. Both map every entry of the record they are handed into the runtime's variable record, so an entry whose value isundefinedplants a key holdingundefined.Variable lookup is presence-keyed.
varResolverreturns onname in variablesbefore it reaches the inline record, the:rootrecord or thevar()fallback:So a key with no value reads as "this variable is set to nothing", and the property it feeds is dropped rather than resolved. Measured on
main(f70c402), with--my-vardefined twice so the compiler cannot inline it and the value is genuinely read at runtime:main<Provider value={{"--my-var": "red"}}>›<Provider value={{"--my-var": undefined}}>›.test { color: var(--my-var) }{ color: "red" }{}:root { --my-var: green },<Provider value={{"--my-var": undefined}}>›.test { color: var(--my-var) }{ color: "green" }{}<Provider value={{"--my-var": undefined}}>›.test { color: var(--my-var, green) }{ color: "green" }{}vars({ "--defined": "red", "--absent": undefined })A single definition of the variable hides all of this: the compiler folds it into the consuming declaration and no runtime
var()read happens, so the property is correct for a reason unrelated to the variable record.Where it belongs
The compiler already holds this invariant one plane over:
No compiled rule can carry a variable entry without a value — a
--x: initialwhose value computes to nothing drops out of the emitted sheet entirely rather than being emitted as an empty entry. The two runtime channels into the same record are the ones that were missing the guard.assignInheritedVariableslooks like the shared choke point, and is not: it writestarget[name] = valueunconditionally and sits only on the descendant path, while avars()object reaches an element's own scope throughrules.ts→calculate-props.ts'sObject.assign(inlineVariables, rule). Fixing it there would leave the reported case unchanged.The change
Both call sites built their entry list with the same expression:
so this is one
toVariableRecord, placed besideVAR_SYMBOLandVariableContextValueinsrc/native/reactivity.ts— the module both call sites already import from, so no new import edge. It strips the--prefix and skips an entry with no value.Absence is how a record spells "no value".
"unset"remains how a variable is deliberately cleared, and is covered by a test so the drop cannot widen into one.Tests
Five cases, each confirmed red against unfixed code for the reason above and green after. Reverting each call site to its inline form independently turns only that site's cases red; making
toVariableRecorddrop every entry turns the two pre-existingvars/VariableContextProvidercases red as well.The 3 failures are the pre-existing Windows-only
babel-plugin-testermismatches over an unrewritten relativerequire("../View"), identical onmainbefore this change (1048 passed, 1072 total).yarn typecheckandyarn lintare clean.variables.test.tsxand one new case invars.test.tsximport throughreact-native-css/native/ the native module rather thanreact-native-css, because the roottsconfigresolves the package specifier to the web half and the cases are about what the native implementation stores.units.test.tsxandmedia-query.test.tsxalready reach the native surface the same way.Scope
Native only. #423 carries the web half of the same contract —
toCustomPropertiesthere drops an undefined value before it reaches a style object — and the two do not touch the same files.