Repository navigation
[unplugin] stop loading imports from transform - #1941
Open
Om-singhaI wants to merge 2 commits into
Open
Om-singhaI wants to merge 2 commits into
Om-singhaI wants to merge 2 commits into
Conversation
In watch mode, transform awaited this.load() on every import of a StyleX file. That never finishes when two StyleX files import each other (vite build --watch, rollup -w), and on a cold start of the Vite dev server it can wait on the dependency optimizer, which is waiting on this transform (meta#1912). The walk also did nothing useful: it read meta.stylex, which transform never returned. transform now returns meta: { stylex: rules }, so shouldTransformCachedModule can restore the rules of a cached module. That hook now also skips runtimeInjection builds and ignores meta without a stylex array.
|
@Om-singhaI is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
meta-cla
Bot
added
the
CLA Signed
There was a problem hiding this comment.
🟡 Changes recommended
Cached-module restoration does not synchronize shared and disk-backed rule stores, allowing stale or missing CSS.
1 open finding
What changed in this PR
Prevents watch-mode deadlocks by removing import loading from transforms and restoring cached StyleX rules through module metadata.
Changes:
- Returns StyleX rules in transform metadata.
- Restores cached rules without loading imports.
- Adds watch-mode and cache-restoration regression tests.
| File | Description |
|---|---|
src/core.js |
Removes import walking and adds metadata-based restoration. |
__tests__/unplugin.test.js |
Covers deadlocks and cached-rule restoration. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+472
to
+476
| if (!stylexOptions.runtimeInjection && Array.isArray(meta?.stylex)) { | ||
| if (meta.stylex.length > 0) { | ||
| stylexRulesById.set(id, meta.stylex); | ||
| } else { | ||
| stylexRulesById.delete(id); |
This branch has not been deployed
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.
What changed / motivation ?
In watch mode, which includes the Vite dev server,
transformincore.jsparsed its own output and awaitedthis.load()on every import before returning. Loading an import can itself wait on this transform, so in some setups it never returns:vite build --watchandrollup -wwhen two StyleX files import each other. Vite 7 exits with "Unexpected early exit ... Unfinished hook action(s)", and Vite 8 and Rollup just hang.vitedev on a cold start (--forceor no dependency cache). On Vite 5 the first request never gets a response, which is [unplugin] Vite dev: import walk calls this.load() on dependencies (react), deadlocking the cold-start dependency optimizer #1912. On Vite 7 and 8 I saw the same hang now and then.The walk also never found what it was looking for. It read
meta.stylexfrom each loaded import, buttransformnever returnedmeta.shouldTransformCachedModulereads the same field, so it did nothing either. On a cold Vite 7 load of a 300 component app the walk also caused 909 transforms of prebundled dependencies, against 7 without it.transformnow returnsmeta: { stylex: rules }, soshouldTransformCachedModulecan restore a cached module's rules. That hook now skipsruntimeInjectionbuilds liketransformdoes, and ignores meta without astylexarray.load()never resolves, and a cached module whose rules come back from its meta.Linked PR/Issues
Fixes #1912
Additional Context
Tests, on Node 25.6.1 and jest 30.1.3 from
packages/@stylexjs/unplugin:npx jest: 2 suites, 26 tests passed.src/core.jsrestored from main and the new tests kept, both new tests fail. The first times out after 1 s and the second gets''for the CSS.core.jshas no@flowpragma.I also built the plugin from main and from this branch into small apps on
@stylexjs/stylex0.19.1:vite --force, the repro from #1912vite --force, 40 cold startsvite build --watch, two files importing each otherrollup -c -wwith@stylexjs/unplugin/rollup, samebuild --watchwithout a cycle, then edit one filecache@stylexjs/rollup-pluginhas the same walk and the samerollup -whang when two StyleX files import each other. I left it out to keep this PR to the unplugin, and can send the same change there separately.The
packagesandprettierchecks are failing on main right now because of the border radius test that #1907 fixes.Pre-flight checklist
Contribution Guidelines