Skip to content

fix(native): a custom property with no value produces no key - #431

Open
YevheniiKotyrlo wants to merge 1 commit into
nativewind:mainfrom
YevheniiKotyrlo:fix/vars-undefined-key
Open

fix(native): a custom property with no value produces no key#431
YevheniiKotyrlo wants to merge 1 commit into
nativewind:mainfrom
YevheniiKotyrlo:fix/vars-undefined-key

Conversation

@YevheniiKotyrlo

Copy link
Copy Markdown
Contributor

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 is undefined plants a key holding undefined.

Variable lookup is presence-keyed. varResolver returns on name in variables before it reaches the inline record, the :root record or the var() fallback:

// src/native/styles/variables.ts
if (name in variables) {
  renderGuards?.push(["v", name, variables[name]]);
  return resolve(variables[name]); // resolve(undefined) === undefined
}

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-var defined twice so the compiler cannot inline it and the value is genuinely read at runtime:

Tree Contract 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 }) one key two keys

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:

// src/compiler/stylesheet.ts
if (value === undefined) {
  return;
}
...
} else if (property.startsWith("--")) {
  rule.v ??= [];
  rule.v.push([property.slice(2), value]);
}

No compiled rule can carry a variable entry without a value — a --x: initial whose 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.

assignInheritedVariables looks like the shared choke point, and is not: it writes target[name] = value unconditionally and sits only on the descendant path, while a vars() object reaches an element's own scope through rules.tscalculate-props.ts's Object.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:

Object.fromEntries(
  Object.entries(variables).map(([k, v]) => [k.replace(/^--/, ""), v]),
)

so this is one toVariableRecord, placed beside VAR_SYMBOL and VariableContextValue in src/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 toVariableRecord drop every entry turns the two pre-existing vars / VariableContextProvider cases red as well.

Test Suites: 2 failed, 4 skipped, 53 passed, 55 of 59 total
Tests:       3 failed, 21 skipped, 1053 passed, 1077 total

The 3 failures are the pre-existing Windows-only babel-plugin-tester mismatches over an unrewritten relative require("../View"), identical on main before this change (1048 passed, 1072 total). yarn typecheck and yarn lint are clean.

variables.test.tsx and one new case in vars.test.tsx import through react-native-css/native / the native module rather than react-native-css, because the root tsconfig resolves the package specifier to the web half and the cases are about what the native implementation stores. units.test.tsx and media-query.test.tsx already reach the native surface the same way.

Scope

Native only. #423 carries the web half of the same contract — toCustomProperties there drops an undefined value before it reaches a style object — and the two do not touch the same files.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant