-
Notifications
You must be signed in to change notification settings - Fork 372
Fix Vitest not compiling external package styles with babel-plugin #1405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix Vitest not compiling external package styles with babel-plugin #1405
Conversation
- Add automatic Vitest deps.inline configuration for external packages - Fix Windows build issue by replacing cp -r with cross-platform Node.js solution - Add isFromStylexPackage helper to transform external package files - Add test case to verify Vitest configuration fix - Update VS Code settings for Flow development Fixes issue where external packages using StyleX were not being compiled during Vitest test runs, causing runtime errors.
…tion This issue is caused by Vitest skipping Babel transforms for external dependencies. Since StyleX requires Babel compilation, externalPackages should automatically inline those dependencies during tests when test: true is enabled. Changes: - Add test option to plugin configuration (defaults to false) - Automatically configure Vitest test.deps.inline for external packages when test: true - Only applies when test: true AND Vitest config detected AND external packages exist - Safely merges with existing user config (preserves existing deps.inline values) - Add comprehensive documentation for test and externalPackages options - Update test to verify the fix works correctly This aligns the test pipeline with the build pipeline without changing runtime behavior or breaking runtime guarantees. Improves developer experience by removing the need for manual deps.inline configuration. Fixes: External packages using StyleX now work in Vitest without runtime errors.
|
Hi @Abhijrathod! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
nmn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of small issues and lot of code that seems redundant.
Generally, we expect you to manually configure vite and vitest to compile external packages that use StyleX. We might improve this in the future, but this PR doesn't seem to be making much of an improvement on that front.
It has a misunderstanding of what stylexPackages even means, which is the imports for StyleX itself and not other external packages that use StyleX.
| "recommendations": [ | ||
| "flowtype.flow-for-vscode", | ||
| "dbaeumer.vscode-eslint", | ||
| "esbenp.prettier-vscode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't include unrelated changes.
| { | ||
| "javascript.validate.enable": false | ||
| } | ||
| "javascript.validate.enable": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
| "license": "MIT", | ||
| "scripts": { | ||
| "prebuild": "rimraf lib && gen-types -i src/ -o lib/cjs && cp -r lib/cjs lib/es", | ||
| "prebuild": "rimraf lib && gen-types -i src/ -o lib/cjs && node -e \"require('fs').cpSync('lib/cjs', 'lib/es', {recursive: true, force: true})\"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want inline node scripts
| - `externalPackages`: array of package names that use StyleX. The plugin auto-discovers packages with StyleX dependencies, but you can manually specify additional packages here. These packages are excluded from Vite dependency optimization. When `test: true` is set, they are automatically added to Vitest's `test.deps.inline` to ensure they are transformed during test runs. | ||
| - `test`: boolean, enables test mode optimizations. When `true` and Vitest is detected, automatically configures `test.deps.inline` for external packages to prevent pre-bundling and ensure StyleX code is compiled. This fixes the issue where external packages using StyleX fail in Vitest with "Unexpected 'stylex.defineVars' call at runtime" errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplication.
| // - /node_modules/@stark-bp/stylex/... | ||
| // - /node_modules/@stark-bp/react/... | ||
| // - /path/to/@stark-bp/stylex/... (for pnpm or symlinked packages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is @stark-bp?
| // - /path/to/@stark-bp/stylex/... (for pnpm or symlinked packages) | ||
| return stylexPackages.some((pkg) => { | ||
| // Escape special regex characters in package name | ||
| const escapedPkg = pkg.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file path matching logic is overly complicated.
Description
This issue is caused by Vitest skipping Babel transforms for external dependencies. Since StyleX requires Babel compilation,
externalPackagesshould automatically inline those dependencies during tests whentest: trueis enabled.Problem
When running Vitest with
@stylexjs/unplugin, external packages that use StyleX are not being compiled by the Babel plugin, causing runtime errors:Error: Unexpected 'stylex.defineVars' call at runtime. Styles must be compiled by '@stylexjs/babel-plugin'.
This happens because Vitest pre-bundles dependencies, skipping Babel transforms for external packages.
Solution
testoption to plugin configuration (defaults tofalse)test: trueis set and Vitest config is detected, automatically configuretest.deps.inlinefor external packagesdeps.inlinevalues)Changes
packages/@stylexjs/unplugin/src/index.js: Added test option and Vitest deps.inline configurationpackages/@stylexjs/unplugin/__tests__/unplugin.test.js: Added test case to verify the fixpackages/@stylexjs/unplugin/README.md: Documentedtestand updatedexternalPackagesoptionspackages/@stylexjs/stylex/package.json: Fixed Windows build issue (cross-platform file copy)Testing
✅ Local StyleX code works
✅ External StyleX packages now work (with fix)
✅
externalPackagesalone is sufficient (no manualdeps.inlineneeded)✅ No runtime
stylex.defineVarserror✅ All existing tests pass
What This Fix Does
deps.inlineconfigurationRelated Issues
Fixes #1399