-
Notifications
You must be signed in to change notification settings - Fork 111
fix(testing-library): do not transform css imports #1500
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
Conversation
🦋 Changeset detectedLatest commit: 9deab2b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds a changeset for @lynx-js/react, restricts the vitest transform plugin to JS/TS files, updates a test import path, and adds CSS and CSS-module test assets plus a new test exercising CSS Modules rendering. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.changeset/curvy-dragons-appear.md (1)
1-5: Nit: make the changeset summary clearer for release notes.Be explicit that we’re skipping CSS transforms in Vitest to fix CSS Modules import errors.
Apply:
-fix css transform error in testing library +testing-library: skip transforming CSS imports in Vitest to fix CSS Modules import errorpackages/react/testing-library/src/vitest.config.js (2)
65-75: Hoist the regex to avoid re-allocating it on every transform callVery minor perf/readability nit: define the regex once per plugin instance instead of recreating it per file transform.
Apply this diff:
function transformReactLynxPlugin() { - return { + // Same regex as rspack's `CHAIN_ID.RULE.JS` + const JS_EXT_RE = /\.(?:js|jsx|mjs|cjs|ts|tsx|mts|cts)(\?.*)?$/; + return { name: 'transformReactLynxPlugin', enforce: 'pre', transform(sourceText, sourcePath) { const id = sourcePath; - // Only transform JS files - // Using the same regex as rspack's `CHAIN_ID.RULE.JS` rule - const regex = /\.(?:js|jsx|mjs|cjs|ts|tsx|mts|cts)(\?.*)?$/; - if (!regex.test(id)) return null; + // Only transform JS files + if (!JS_EXT_RE.test(id)) return null;
69-75: Optionally ignore virtual module ids earlySlight hardening: skip Vite/Rollup virtual modules (ids starting with “\0”) explicitly. It’s harmless here but keeps intent clear and avoids touching synthetic ids.
Apply this diff:
transform(sourceText, sourcePath) { const id = sourcePath; + // Ignore virtual modules + if (id[0] === '\0') return null; // Only transform JS files
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/curvy-dragons-appear.md(1 hunks)packages/react/testing-library/src/__tests__/css/index.test.jsx(2 hunks)packages/react/testing-library/src/__tests__/css/style1.css(1 hunks)packages/react/testing-library/src/__tests__/css/style2.css(1 hunks)packages/react/testing-library/src/__tests__/css/style3.module.css(1 hunks)packages/react/testing-library/src/vitest.config.js(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.508Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/curvy-dragons-appear.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/curvy-dragons-appear.md
📚 Learning: 2025-08-07T04:00:59.627Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/curvy-dragons-appear.md
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-11T06:00:04.354Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/react/testing-library/src/plugins/vitest.ts:59-61
Timestamp: 2025-08-11T06:00:04.354Z
Learning: In the lynx-family/lynx-stack repository, the `testingLibraryPlugin` in `packages/react/testing-library/src/plugins/vitest.ts` intentionally uses `process.exit` when jsdom installation fails, maintaining consistency with the previous implementation from `packages/react/testing-library/src/vitest.config.js`. This behavior should not be changed to use `this.error` despite being a Vite plugin best practice.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-11T05:59:28.508Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.508Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
packages/react/testing-library/src/vitest.config.js
🔇 Additional comments (5)
packages/react/testing-library/src/__tests__/css/style2.css (1)
1-3: LGTM: simple CSS fixture for import side-effects.This is a minimal, clear asset for exercising CSS imports in tests. No issues.
packages/react/testing-library/src/__tests__/css/style1.css (1)
1-3: LGTM: concise CSS fixture.Appropriate for validating that plain CSS imports don’t break under the new transform behavior.
packages/react/testing-library/src/__tests__/css/style3.module.css (1)
1-3: LGTM: CSS Module fixture aligns with test usage.Class name matches the test expectations (style3.baz). No concerns.
packages/react/testing-library/src/__tests__/css/index.test.jsx (1)
5-7: CSS/CSS Module handling is correctly delegated to Vite
- The transform hook in packages/react/testing-library/src/vitest.config.js (lines 69–76) uses a JS/TS-only regex (
\.(?:js|jsx|mjs|cjs|ts|tsx|mts|cts)(\?.*)?$), returningnullfor CSS files.- No direct
import … from 'vite'statements are present in this package.packages/react/testing-library/src/vitest.config.js (1)
71-74: LGTM: gating the transform to JS/TS correctly avoids transforming CSSReturning null for non-JS/TS ids delegates CSS/CSS-Modules to Vite’s CSS pipeline, aligning with the PR objective and fixing the prior CSS transform issue. Using the rspack-aligned regex is a nice consistency touch.
packages/react/testing-library/src/__tests__/css/index.test.jsx
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Yiming Li <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Yiming Li <[email protected]>
CodSpeed Performance ReportMerging #1500 will not alter performanceComparing Summary
|
React Example#4191 Bundle Size — 237.04KiB (0%).9deab2b(current) vs cb6d23c main#4175(baseline) Bundle metrics
|
| Current #4191 |
Baseline #4175 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
158 |
158 |
|
64 |
64 |
|
45.86% |
45.86% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #4191 |
Baseline #4175 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.28KiB |
91.28KiB |
Bundle analysis report Branch upupming:fix/testing-css-common Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4186 Bundle Size — 344.52KiB (0%).9deab2b(current) vs cb6d23c main#4171(baseline) Bundle metrics
|
| Current #4186 |
Baseline #4171 |
|
|---|---|---|
143.37KiB |
143.37KiB |
|
31.84KiB |
31.84KiB |
|
0% |
41.61% |
|
7 |
7 |
|
7 |
7 |
|
211 |
211 |
|
17 |
17 |
|
3.96% |
3.96% |
|
4 |
4 |
|
0 |
0 |
Bundle size by type no changes
| Current #4186 |
Baseline #4171 |
|
|---|---|---|
229.72KiB |
229.72KiB |
|
82.95KiB |
82.95KiB |
|
31.84KiB |
31.84KiB |
Bundle analysis report Branch upupming:fix/testing-css-common Project dashboard
Generated by RelativeCI Documentation Report issue
colinaaa
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.
👍
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - fix css transform error in testing library ([#1500](#1500)) - fix the type error of `wrapper` option in testing library's `render` and `renderHook` function ([#1502](#1502)) - Introduce recursive hydration for lists to prevent double remove/insert on moves. ([#1401](#1401)) - Handle `<frame/>` correctly. ([#1497](#1497)) ## @lynx-js/[email protected] ### Patch Changes - `output.inlineScripts` defaults to `false` when chunkSplit strategy is not `'all-in-one'` ([#1504](#1504)) ## @lynx-js/[email protected] ### Patch Changes - `output.inlineScripts` defaults to `false` when chunkSplit strategy is not `'all-in-one'` ([#1504](#1504)) - Updated dependencies \[[`51a0b19`](51a0b19), [`b391ef5`](b391ef5)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix that `lynxTestingEnv.jsdom` cannot be initialized correctly when `global.jsdom` is not defined. ([#1422](#1422)) ## @lynx-js/[email protected] ### Patch Changes - fix: systeminfo in mts function ([#1537](#1537)) - feat: add MTS API: \_\_UpdateComponentInfo ([#1485](#1485)) - fix: `__ElementFromBinary` needs to correctly apply the dataset in elementTemplate to the Element ([#1487](#1487)) - fix: all attributes except `id` and `type` under ElementTemplateData are optional. ([#1483](#1483)) - feat: add MTS API \_\_GetAttributeByName ([#1486](#1486)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: systeminfo in mts function ([#1537](#1537)) - refactor: use utf-8 string ([#1473](#1473)) - Fix mtsGlobalThis race condition in createRenderAllOnUI ([#1506](#1506)) - Updated dependencies \[[`405a917`](405a917), [`b8f89e2`](b8f89e2), [`f76aae9`](f76aae9), [`b8b060b`](b8b060b), [`d8381a5`](d8381a5), [`214898b`](214898b), [`ab8cee4`](ab8cee4)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: use utf-8 string ([#1473](#1473)) ## @lynx-js/[email protected] ### Patch Changes - fix: systeminfo in mts function ([#1537](#1537)) - refactor: use utf-8 string ([#1473](#1473)) - feat: add MTS API: \_\_UpdateComponentInfo ([#1485](#1485)) - fix: \_\_ElementFromBinary should mark all elements actively ([#1484](#1484)) - fix: `__ElementFromBinary` needs to correctly apply the dataset in elementTemplate to the Element ([#1487](#1487)) - fix: all attributes except `id` and `type` under ElementTemplateData are optional. ([#1483](#1483)) - feat: add MTS API \_\_GetAttributeByName ([#1486](#1486)) - Updated dependencies \[[`405a917`](405a917), [`b8f89e2`](b8f89e2), [`f76aae9`](f76aae9), [`d8381a5`](d8381a5), [`214898b`](214898b), [`ab8cee4`](ab8cee4)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: use utf-8 string ([#1473](#1473)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`405a917`](405a917), [`b8f89e2`](b8f89e2), [`f76aae9`](f76aae9), [`b8b060b`](b8b060b), [`d8381a5`](d8381a5), [`214898b`](214898b), [`ab8cee4`](ab8cee4)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix "emit different content to the same filename" error ([#1482](#1482)) ## @lynx-js/[email protected] ### Patch Changes - Fix invalid `module.exports=;` syntax in app-service.js. ([#1501](#1501)) ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Notes
Checklist