fix(native): rank an unset specificity slot the same however it is spelled - #432
Open
YevheniiKotyrlo wants to merge 1 commit into
Open
fix(native): rank an unset specificity slot the same however it is spelled#432YevheniiKotyrlo wants to merge 1 commit into
YevheniiKotyrlo wants to merge 1 commit into
Conversation
…elled
A specificity array is sparse. A rule that sets `PseudoElements` never writes
`Important` or `Inline`, so those sit as holes inside the array's length —
`selector-builder.ts` merges with `if (value !== undefined)` and
`stylesheet.ts` skips an absent spec entirely, so nothing fills them in.
A hole reads as `undefined` in memory. The sheet reaches a native runtime
through `JSON.stringify` (`metro/injection-code.ts`), and JSON has no holes, so
every one arrives as `null`.
`specificityCompareFn` branched on the RAW slot while returning a NORMALISED
difference:
if (aSpec[Important] !== bSpec[Important]) {
return (aSpec[Important] || 0) - (bSpec[Important] || 0);
`undefined !== null` is true, so the comparison entered that branch and answered
`0 - 0`, settling at a slot neither rule uses and never reaching the one that
decides. Two rules that differ only in whether they carry a pseudo-element
compare equal.
The caller is the runtime sort in `native/styles/index.ts`, over rules gathered
across every class name on the element. A zero verdict leaves it nothing to
order by, so the `className` attribute's token order decides the cascade:
className="inp inp-ph" -> one result
className="inp-ph inp" -> the other
`placeholder:` and `selection:` are the everyday Tailwind triggers, and they are
the only two pseudo-elements this compiler emits.
Comparing the ranked value rather than the raw slot fixes it. The loop is part
of that: returning inside a raw-slot branch is what made a `0` difference
terminal instead of falling through to the next slot.
Nothing else reads these slots at runtime — every other `Specificity.` read is
compile time, where the array still has its holes and is already correct. That
is also why the existing suite is blind to this: the compile-time sort runs on
the in-memory form, and it masks the runtime bug whenever two rules share a
class name.
The test asserts at the comparator, over a sheet put through the JSON round trip
a device receives, rather than through a render. A rendered assertion would need
a non-`color` declaration to leak out of the pseudo-element rule, so it would go
inert the moment that leak is fixed; this one does not.
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
specificityCompareFncan return0for two rules that are not equally specific, once the stylesheet has been through Metro.A specificity array is sparse. A rule that sets
PseudoElementsnever writesImportantorInline—selector-builder.tsmerges withif (value !== undefined), andstylesheet.tsskips an absent spec entirely — so those slots are holes inside the array's length:A hole reads as
undefinedin memory.getNativeInjectionCode(src/metro/injection-code.ts) writes the sheet withJSON.stringify, and JSON has no holes, so a device receives:The comparator branches on the raw slot while returning a normalised difference:
undefined !== nullis true, so the comparison enters that branch and answers0 - 0— settling at a slot neither rule uses, and never reaching the one that decides.Why it is visible
The caller is the runtime sort in
src/native/styles/index.ts, over rules gathered across every class name on the element. A0verdict leaves it nothing to order by, so theclassNameattribute's token order decides the cascade:placeholder:andselection:are the everyday Tailwind triggers, and::selection/::placeholderare the only two pseudo-elements this compiler emits.The compile-time sort in
src/compiler/stylesheet.tsruns on the in-memory form, where the holes are still holes, and is correct. That is also why the current suite does not catch this: the compile-time sort masks it whenever two rules share a class name, and only the runtime sort — over a cross-class set — is exposed.Fix
Compare the ranked value rather than the raw slot:
The loop is part of the fix, not cosmetic. Returning inside a raw-slot branch is what made a
0difference terminal instead of falling through to the next slot; that behaviour is reproducible by restoring the early return inside the loop, and it is what the new test pins.Scope
This touches the comparator only. Every other
Specificity.read is compile time (selector-builder.ts,stylesheet.ts), where the array still has its holes and is already correct — there are no.s[reads anywhere undersrc/native. Nothing else needs to change.Test
src/__tests__/native/specificity.test.tsxgains one case. It asserts at the comparator, over a sheet put through the sameJSON.stringifyround trip a device receives, and covers both directions plus the inline-record fallback (inlineSpecificityis itself a sparse array, so an inline style must still win).Measured on this branch: red at
Received: 0, green after.A rendered test would be the more obvious choice and is deliberately not what this uses. Making the tie visible in rendered output needs a non-
colordeclaration to leak out of the pseudo-element rule, so a rendered assertion would go inert as soon as that leak is fixed. The comparator assertion does not depend on it.Verification
3 failed, 21 skipped, 1049 passed, 1073 total. The three aresrc/__tests__/babel/*path suites, unrelated to this change and pre-existing on this base — they fail identically withsrc/utilities/specificity.tsreverted.yarn typecheckandyarn lintboth clean.JSON.stringifyform — half of every pair where exactly one rule carries a pseudo-element.