fix(native): let an element's own custom property outrank an inherited one - #443
Open
YevheniiKotyrlo wants to merge 1 commit into
Open
fix(native): let an element's own custom property outrank an inherited one#443YevheniiKotyrlo wants to merge 1 commit into
YevheniiKotyrlo wants to merge 1 commit into
Conversation
…d one
`varResolver` read the inherited variable context before the element's own
record and returned from it, so a `vars()` declaration on the element was used
only when no ancestor declared the same name.
css-cascade-4 §7.2 makes inheritance a defaulting step: an element inherits a
property only when the cascade yields no declared value for it. A custom
property is an ordinary property (css-variables-1 §2), so an element declaring
`--x` uses its own value rather than its ancestor's.
Measured against a provider declaring `--my-var: red` around an element whose
`vars()` declares `--my-var: blue`, with the class reading `color: var(--my-var)`:
expected { color: "blue" }
actual { color: "red" }
Web is unaffected and already correct — it implements no precedence of its own.
`vars()` returns a plain `{"--x": v}` style object and `VariableContextProvider`
renders a `display: contents` div carrying the property, so the browser's cascade
decides.
The inherited read moves below the element's own record and keeps pushing the
RAW descriptor, which is what `testGuards` compares against the next render's
context. The second read of `variables[name]` that followed it is deleted: it
ran only when `name` was absent from `variables`, so it resolved `undefined` and
its guard never fired.
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.
Problem
varResolverreads the inherited variable context before the element's own record and returns from it, so avars()declaration on the element is used only when no ancestor declared the same name. Put a<VariableContextProvider>anywhere above the element and its own declaration is discarded.css-cascade-4 §7.2 makes inheritance a defaulting step: an element inherits a property only when the cascade yields no declared value for it. A custom property is an ordinary property (css-variables-1 §2), so an element that declares
--xuses its own value, never its ancestor's.Reproduction
--my-varis declared twice soinline-variables.tscannot fold it into the consuming declaration — a single definition performs no runtimevar()read and would pass without exercising precedence at all.{ color: "blue" }main{ color: "red" }Only this case diverges. I probed seven trees —
vars()alone, provider alone, provider above a parent with a reading child,vars()on a parent with a reading child, two siblings under one provider, two siblings with their ownvars()— and every other one already matches the expected value.Which planes
Web is unaffected and already correct. It implements no precedence of its own:
vars()returns a plain{"--x": v}style object andVariableContextProviderrenders adisplay: contentsdiv carrying the property, so the browser's cascade decides. I measured the same tree in chromium — the element's own declaration wins.The compiler is unaffected; it emits
v/vrtuples and never orders the two channels.Fix
The inherited read moves below the element's own record. It keeps pushing the raw descriptor rather than the resolved value, because
testGuardscompares the guard against the next render's context and a resolved value would never match.The second read of
variables[name]that followed the inline arm is deleted. It was unreachable-effective: it ran only whennamewas absent fromvariables, soresolve(variables[name])wasresolve(undefined)and itsvalue !== undefinedguard never fired.Net: 6 insertions, 12 deletions in one function.
Tests
Two added to
src/__tests__/native/vars.test.tsx:vars()outranks an inherited value — fails onmainwith- "color": "blue"/+ "color": "red", passes with the fix;Validation
yarn test— 1050 passed.yarn lintandyarn typecheckclean.src/__tests__/babel/react-native.test.tsandreact-native-web.test.tsreport 3 failures on module-specifier resolution. Those are pre-existing onmain: I measured the baseline by swapping onlyvariables.tsback tomain's copy and got the same 3. They look like a Windows path-separator issue and are unrelated to this change.