Last 12 weeks · 18 commits
5 of 6 standards met
Context The plugin evaluates style values at compile time. That evaluator did not limit which values it was allowed to reach, so it could walk from an ordinary object up to built-in constructors and then call them. Compiling a file could therefore execute code from that file at build time. That is harmless when you compile your own source. It is not harmless anywhere the plugin compiles input the author does not control. Implementation Restrict the evaluator to what it actually needs: Refuse prototype chain access on evaluated values. Normalize computed keys before checking them, so a disguised key cannot slip past the check and then resolve anyway. Allow a fixed set of built-in static methods, replacing a denylist that could never be complete. Look up the plugin's own config by own keys only, so nothing resolves through inheritance. Backstop: refuse to invoke anything that turns strings into code, or that re-points a function at a new receiver, whatever route it arrives by. A callee that is not callable now bails out instead of throwing out of the compiler. Behavior change A few reflective and prototype related built-in methods are no longer evaluated at compile time and bail out instead. Nothing in the repo used them. Tests 19 added. The important one asserts that a payload never runs, rather than that each expression bails out, so it holds however the evaluator refuses. Checked 61 expressions covering every route I could find. 17 of them execute code on . None execute here. Full suite green, snapshots unchanged, Flow, eslint and prettier clean. Another issue surfaced: Out of scope here, but computed calls resolve the wrong property, using the name of the variable holding the key rather than its value. Reads resolve it correctly. This predates the change, and is a silent miscompile rather than a safety issue.
Fixes #1786 What changed / motivation ? reports every key of a object as unused when the object is exported using the specifier form: The rule already exempts and , on the basis that an exported styles object may have its keys used by another module. and are the same situation but were not covered. The rule is , so acted on the false positive and deleted the style keys from the source file, emptying the call in the example above to . Cause: the visitor only looked at . For that is and the exported bindings live in , so the variable was never removed from and reported all of its keys. The fix iterates and exempts each name. Re-exports () are skipped via the guard, because they export another module's binding and must not exempt a same-named local variable. Linked PR/Issues Fixes # (issue) Additional Context Tests added to : valid: valid: valid: (specifier list with an unrelated binding) valid: invalid: — still reports the locally unused key, covering the re-export guard No documentation change: neither the plugin README nor the docs site describes the rule's export handling. Pre-flight checklist [x] I have read the contributing guidelines Contribution Guidelines [x] Performed a self-review of my code
fix(unplugin): match hashed index and style CSS asset names What changed / motivation ? in chooses which stylesheet the collected StyleX rules get appended to. Its documented contract is to prefer a stylesheet named like or , and to fall back to the first CSS asset only when neither exists. The two patterns it used required the file name to end in exactly or : Vite's default is , so the entry stylesheet reaches the bundle as . Neither pattern can match a hashed name, so is always in a production Vite or Rollup build and the picker silently degrades to , which is whichever stylesheet the bundler happened to emit first. The documented preference never runs at all. When that first stylesheet is not the entry stylesheet, the StyleX rules are appended to a file the entry page never loads, and the styles simply do not apply in the production build. The same two expressions were duplicated across every bundler backend, so the same miss occurred in the output directory fallback in and , in the hook in , and in both picking sites in . Twelve regex literals in five files, all with the same defect. Fixes #1815 Additional Context Reproduction A two entry Vite 8.2.2 build, default config, no . The CSS assets arrive in the bundle in this order: is the admin stylesheet, so that is where the StyleX CSS landed. Building the same project with the plugin compiled from and then from this branch, and grepping the output for a marker rule: Worth stating precisely, since the issue describes a lazy route chunk: on Vite 8.2.2 a single entry app with a dynamically imported route emitted its CSS in the order , so was the entry stylesheet by luck and the bug stayed hidden. The multi entry build above is the case I could reproduce end to end. Either way the underlying defect is the same: the preference is dead code under default asset naming, and the result depends on bundler emission order rather than on the documented rule. The fix Both patterns now allow an optional content hash, and are defined once and shared by all five backends: The character class has to be the full base64url alphabet, not just letters and digits. Rollup's default is , and the hash is produced by , so roughly one eight character hash in five contains a or a . Sampling 20000 hashes straight from Rollup 4.59's own hasher gives 4082 such hashes, and a narrower class misses every one of them: That is not a hypothetical. Varying the entry CSS in the two entry app above until its hash lands on such a value gives a real Vite 8.2.2 build where the narrower pattern reproduces the original bug exactly: Building that app with the plugin compiled each way, and grepping the output for the marker rule, shows the difference is not cosmetic. A hash class of letters and digits alone leaves the entry stylesheet empty exactly as does: , a few lines below in the same file, uses the narrower shape. It is left as it is on purpose: it strips a trailing hash off a name with a lazy prefix, and widening its class there would make it over strip, turning into instead of . Matching a whole name and stripping a suffix want different classes, so they are deliberately not shared. The flag covers Vite's mixed case hashes such as . The leading anchor is kept deliberately. Without it a name such as would be treated as the entry stylesheet. There is a test covering that. Unhashed and still match through the optional group, and an explicit still takes priority over both, so existing behaviour is unchanged. Tests Eight tests were added under a new block in . Every hashed file name used in them was taken from real output rather than invented. Five of them fail on and pass on this branch: That failure output was captured by reverting only and keeping the new tests in place. The two underscore and hyphen cases also fail against a version of this fix that uses a plain hash class, which is what pins the character class down rather than leaving it to taste. The other three tests guard behaviour that was already correct and must stay correct: unhashed still wins, a longer name ending in is not mistaken for the entry, and still overrides both patterns. With the fix applied, the full package suite passes: and are both clean on the changed files. The changed files carry no pragma, so Flow does not type check them. Scope note and were updated along with the Vite and Rollup paths. Hashed CSS names are opt in rather than default for those two bundlers, so they are not what issue #1815 reports, but they carried literal copies of the same two expressions and would miss a hashed name in exactly the same way. Leaving half the backends on the old patterns while sharing constants in the other half seemed worse than making the behaviour uniform. Happy to split that into a separate change if preferred. Known limits Two things this deliberately does not cover, flagged so they are a choice rather than an oversight. Webpack users commonly configure , with a dot rather than a hyphen before the hash. That still will not match. Accepting as the separator would cover it, at the cost of claiming names like as the entry stylesheet, so it is left out until someone actually reports it. Widening the class to base64url does let a hand written name such as match. That is a heuristic replacing another heuristic: the alternative for such a bundle is , whichever stylesheet the bundler emitted first, and a file literally named is the better of the two guesses. remains the escape hatch for anyone who wants to name the target outright. Preflight checklist [x] I have read the contributing guidelines [x] Performed a self review of my code
Repository: facebook/stylex. Description: StyleX is the styling system for ambitious user interfaces. Stars: 9822, Forks: 456. Primary language: JavaScript. Languages: JavaScript (94.7%), TypeScript (3.2%), MDX (2.1%), CSS (0%), HTML (0%). License: MIT. Homepage: https://stylexjs.com Latest release: 0.17.5 (7mo ago). Open PRs: 100, open issues: 118. Last activity: 12h ago. Community health: 87%. Top contributors: nmn, mellyeliu, necolas, dependabot[bot], henryqdineen, nedjulius, skovhus, Prakshal-Jain, nonzzz, dwei-figma and others.