feat: support rstest in testing library #1305
Conversation
🦋 Changeset detectedLatest commit: ff2cb92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 rstest support across the Lynx React ecosystem: new rstest configs, a testing loader and plugins for webpack/rsbuild/rspeedy, vitest compatibility polyfill, env/setup files, rspeedy templates/examples, CI/workflow and workspace catalog entries, plus numerous test snapshot and minor API export adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
packages/react/testing-library/src/__tests__/worklet.test.jsx (1)
80-82: Snapshot strings still hard-code worklet IDs – consider making the expectation less brittleThe only change here is the literal ID prefix (
a45f:→e062:).
Because these IDs are auto-generated they can drift again, producing noisy diffs and flaky CI failures.A more future-proof pattern is to assert on the structural bits that matter and ignore the opaque ID, e.g.
- "data": "{\"patchList\":[{\"snapshotPatch\":[3,-2,0,{\"_wkltId\":\"e062:test:2\", ... + "data": expect.stringContaining('\"snapshotPatch\"'),or split the JSON, parse it and assert on its parsed shape.
Not urgent for functionality, but will save churn.
Also applies to: 162-164, 252-254, 346-348, 461-463
packages/react/testing-library/src/__tests__/act.test.jsx (1)
216-218: Hard-coded snapshot type identifiers risk future churnSame concern as in
worklet.test.jsx: the new snapshot marker__Card__:__snapshot_a6614_test_4is an internal hash that can change for unrelated reasons.If the identifier is not business-critical, prefer:
expect(callLepusMethodCalls[0][1].data) .toEqual(expect.stringContaining('"type":"__Card__'))or parsing the JSON and asserting selected keys.
Also applies to: 251-253, 276-278, 300-302
packages/react/testing-library/package.json (1)
10-11:rstest runscript added – consider unifying test entryWith two test runners (
vitest,rstest) developers must remember which to run.
A combined script, e.g."test:all": "pnpm run test && pnpm run test:rs"can shorten CI configs and local DX.
Also verify that the alias
"catalog:rsbuild"resolves in all registries you publish to.Also applies to: 15-18
packages/react/testing-library/src/plugins/index.ts (1)
1-2: Path extension may confuse TS toolingImporting
./rstest.js/./vitest.jsfrom a TypeScript source works at runtime after build, but some IDEs/linters complain when the physical file is*.ts.
If the sibling files are TypeScript, you can omit the extension and rely on compiler/moduleResolution:export { testingLibraryPlugin as rstestTestingLibraryPlugin } from './rstest';Not blocking, just a DX nicety.
packages/react/testing-library/src/plugins/rstest.ts (1)
25-40: Consider making the API stub version configurable and improve debug logging.The API stub implementation provides necessary functionality, but there are a few areas for improvement:
- The hardcoded version
'0.0.0'should be configurable or derived from package.json- The debug method could use a more appropriate logging level than
console.infoConsider these improvements:
- version: '0.0.0', + version: process.env.npm_package_version || '0.0.0',And for better debug logging:
- console.info(message); + console.debug(message);packages/react/testing-library/vitest.config.ts (1)
2-2: Consider importing from source instead of dist for development.Importing from
./dist/pluginscould cause issues during development if the dist files aren't built. Consider importing from the source files instead.-import { vitestTestingLibraryPlugin } from './dist/plugins'; +import { vitestTestingLibraryPlugin } from './src/plugins';This ensures the import works even when dist files aren't available during development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
.changeset/easy-waves-pump.md(1 hunks).changeset/red-lamps-arrive.md(1 hunks).github/workflows/test.yml(1 hunks)package.json(1 hunks)packages/react/package.json(1 hunks)packages/react/testing-library/package.json(1 hunks)packages/react/testing-library/rslib.config.ts(2 hunks)packages/react/testing-library/rstest.config.ts(1 hunks)packages/react/testing-library/src/__tests__/act.test.jsx(4 hunks)packages/react/testing-library/src/__tests__/auto-cleanup-skip.test.jsx(1 hunks)packages/react/testing-library/src/__tests__/cleanup.test.jsx(1 hunks)packages/react/testing-library/src/__tests__/end-to-end.test.jsx(5 hunks)packages/react/testing-library/src/__tests__/lazy-bundle/index.test.jsx(2 hunks)packages/react/testing-library/src/__tests__/list.test.jsx(16 hunks)packages/react/testing-library/src/__tests__/lynx.test.jsx(1 hunks)packages/react/testing-library/src/__tests__/rerender.test.jsx(1 hunks)packages/react/testing-library/src/__tests__/worklet.test.jsx(5 hunks)packages/react/testing-library/src/plugins/index.ts(1 hunks)packages/react/testing-library/src/plugins/rstest.ts(1 hunks)packages/react/testing-library/src/plugins/vitest.ts(1 hunks)packages/react/testing-library/src/setupFiles/common.js(1 hunks)packages/react/testing-library/src/setupFiles/rstest.js(1 hunks)packages/react/testing-library/src/setupFiles/vitest.js(1 hunks)packages/react/testing-library/src/vitest.config.js(0 hunks)packages/react/testing-library/types/vitest-config.d.ts(0 hunks)packages/react/testing-library/vitest-polyfill.js(1 hunks)packages/react/testing-library/vitest.config.ts(1 hunks)packages/rspeedy/plugin-react/src/entry.ts(1 hunks)packages/rspeedy/plugin-react/src/loaders.ts(1 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(2 hunks)packages/testing-library/examples/basic/package.json(1 hunks)packages/testing-library/examples/basic/rstest.config.ts(1 hunks)packages/testing-library/examples/basic/src/__tests__/index.test.tsx(1 hunks)packages/testing-library/examples/basic/src/rspeedy-env.d.ts(1 hunks)packages/testing-library/examples/basic/vitest.config.ts(1 hunks)packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/options.ts(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/testing.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/react/testing-library/types/vitest-config.d.ts
- packages/react/testing-library/src/vitest.config.js
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/webpack/react-webpack-plugin/src/loaders/options.ts (1)
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/rspeedy/plugin-react/src/pluginReactLynx.ts (1)
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/rspeedy/plugin-react/src/entry.ts (1)
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
🧬 Code Graph Analysis (3)
packages/react/testing-library/src/setupFiles/vitest.js (2)
packages/react/testing-library/src/setupFiles/rstest.js (1)
globalThis(9-11)packages/react/testing-library/src/setupFiles/common.js (1)
globalThis(20-27)
packages/react/testing-library/src/setupFiles/rstest.js (2)
packages/react/testing-library/src/setupFiles/vitest.js (1)
globalThis(1-3)packages/react/testing-library/src/setupFiles/common.js (1)
globalThis(20-27)
packages/react/testing-library/src/plugins/vitest.ts (4)
packages/react/testing-library/src/__tests__/lazy-bundle/index.test.jsx (1)
require(8-8)packages/react/testing-library/src/plugins/index.ts (2)
testingLibraryPlugin(1-1)testingLibraryPlugin(2-2)packages/react/testing-library/src/plugins/rstest.ts (1)
testingLibraryPlugin(3-42)packages/react/testing-library/loaders/jsx-loader.js (4)
runtimePkgName(9-9)sourcePath(10-10)relativePath(12-15)basename(11-11)
🔇 Additional comments (47)
package.json (1)
23-23: @rstest/core version and stability verified
- The latest published version on npm is 0.0.8.
- There are no known security advisories for this package.
- In semver, using
^0.0.8for a 0.x release only allows patch-level updates (>=0.0.8 <0.0.9), so it’s effectively pinned to the current patch line.- If you want absolute immutability, you can change to an exact version:
- "@rstest/core": "^0.0.8", + "@rstest/core": "0.0.8",Otherwise, keeping the caret is acceptable for automatic patch-level fixes.
packages/react/testing-library/src/__tests__/auto-cleanup-skip.test.jsx (1)
1-2: Good practice: Explicit imports improve code clarity.Making the vitest imports explicit (
beforeAll,test,expect) instead of relying on globals is a good practice that improves code maintainability and makes dependencies clear.packages/react/testing-library/src/__tests__/cleanup.test.jsx (1)
1-1: Correct import for vitest mocking utilities.Adding
vito the vitest imports is necessary for the mocking functionality used in the test (vi.fn()on line 6). This maintains consistency with vitest's mocking API.packages/react/testing-library/src/__tests__/rerender.test.jsx (1)
3-3: Necessary import for vitest mocking and spying utilities.Adding
vito the vitest imports is required for the mocking functionality used in this test (vi.spyOn()on line 47 andvi.clearAllMocks()on line 53).packages/react/testing-library/src/__tests__/lynx.test.jsx (1)
1-1: Consistent import for vitest mocking utilities.Adding
vito the vitest imports is necessary for the mocking functionality (vi.fn()on line 5) and maintains consistency with other test files in the suite..changeset/easy-waves-pump.md (1)
1-7: LGTM! Clear and concise changeset documentation.The changeset properly documents the patch updates for both packages and clearly describes the rstest testing framework integration with dedicated testing loader support.
packages/testing-library/examples/basic/src/rspeedy-env.d.ts (1)
2-3: LGTM! Proper ambient type declarations for dual testing framework support.The triple-slash references correctly extend the ambient type environment to include globals from both Vitest and Rstest frameworks, supporting the new dual testing infrastructure.
packages/testing-library/examples/basic/package.json (1)
9-10: LGTM! Clean addition of rstest testing script.The new
test:rsscript follows good naming conventions and provides a clear way to run tests with the rstest framework alongside the existing vitest support.packages/testing-library/examples/basic/src/__tests__/index.test.tsx (1)
20-20: Verify mock function compatibility in basic exampleAutomated search shows that in
packages/testing-library/examples/basic/src/__tests__/index.test.tsx, you assign bothrstest.fnandvi.fntofnand then call:const cb = fn(); expect(cb).toBeCalledTimes(1); expect(cb.mock.calls).toMatchInlineSnapshot(`…`);However, the implementation of
rstest.fnisn’t present in this repo, so please manually confirm that:
rstest.fn()returns a mock object with a.mockproperty exposingcalls,results, etc., identical to Vitest’svi.fn.- All mock assertions (
toBeCalledTimes,.mock.calls,.mock.results) behave the same under both frameworks.Location to verify:
- packages/testing-library/examples/basic/src/tests/index.test.tsx (around the
const cb = fn();line)packages/rspeedy/plugin-react/src/pluginReactLynx.ts (2)
29-29: LGTM! Proper import of testing environment loaders.The import of
applyTestingEnvLoadersis correctly added alongside the existingapplyLoadersimport to support the conditional testing environment logic.
370-380: Environment-specific loader logic verified
applyTestingEnvLoadersis defined and exported inpackages/rspeedy/plugin-react/src/loaders.ts.- The conditional in
pluginReactLynx.tscorrectly applies CSS only outside of tests and swaps inapplyTestingEnvLoaderswhenNODE_ENV === 'test'.No further changes needed.
packages/webpack/react-webpack-plugin/src/loaders/options.ts (1)
17-20: Exporting internal constants – double-check public-API surface
JSX_IMPORT_SOURCEandRUNTIME_PKGare now exported.
Before merging, make sure:
- They’re documented somewhere if considered public.
- Down-stream packages import them from this file instead of duplicating strings (single source of truth).
- No circular-dependency is introduced (nothing inside
options.tsimports from modules that will now import it).Otherwise, looks good.
Also applies to: 22-22
.changeset/red-lamps-arrive.md (1)
7-23: Well-structured rstest configuration example.The configuration example demonstrates proper integration of rstest with the ReactLynx testing library. The setup includes the correct plugins, environment configuration, and setup files.
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts (1)
117-121: Clean implementation of testing loader support.The addition of the
'TESTING'loader key follows the existing pattern and provides the necessary testing-specific transformation capabilities for the ReactLynx webpack plugin.packages/react/testing-library/src/__tests__/list.test.jsx (2)
6-6: Appropriate addition of vitest mocking utilities.The import of
vifrom vitest enables proper mocking functionality for the test suite, aligning with the testing framework integration.
34-34: Snapshot updates reflect underlying implementation changes.The snapshot identifiers have been updated from
__snapshot_a9e46_test_*to__snapshot_f24e5_test_*consistently throughout the test file. This is a routine update that occurs when the underlying testing infrastructure changes affect snapshot generation.Also applies to: 53-53, 63-63, 88-88, 95-95, 105-105, 139-139, 146-146, 156-156, 189-189, 196-196, 203-203, 214-214, 292-292, 474-474, 582-582, 606-606, 618-618, 729-729
packages/react/testing-library/src/__tests__/end-to-end.test.jsx (2)
3-3: Proper addition of vitest mocking utilities.The import of
vifrom vitest enables mocking functionality that's used in the test suite, which is appropriate for the testing framework integration.
70-70: Snapshot updates maintain test integrity.The snapshot identifiers have been updated to reflect changes in the underlying testing infrastructure. These updates are necessary to maintain accurate test assertions while preserving the test logic.
Also applies to: 83-83, 103-103, 121-121
packages/react/testing-library/src/setupFiles/vitest.js (1)
1-20: LGTM! Clean implementation of vitest-specific setup.The setup file correctly preserves existing global functions, implements proper error handling for the dynamic import mechanism, and follows a consistent pattern with the rstest counterpart.
packages/testing-library/examples/basic/vitest.config.ts (1)
1-11: LGTM! Excellent simplification of the vitest configuration.The migration from async config merging to a direct plugin-based approach is a clean architectural improvement. The synchronous configuration is more maintainable and the plugin integration looks correct.
packages/testing-library/examples/basic/rstest.config.ts (1)
1-16: LGTM! Well-structured rstest configuration.The configuration properly sets up the rstest environment with the correct plugins, jsdom environment, and setup files. The use of
require.resolvefor the setup file path ensures proper module resolution.packages/react/testing-library/src/setupFiles/rstest.js (2)
1-7: LGTM! Proper global environment setup for rstest.The initialization of
jsdomandlynxTestingEnvglobals is correctly implemented and follows the expected pattern for test environment setup.
18-21: Override safe: no tests depend on lynx.requireModuleAsyncI checked under
packages/react/testing-libraryand found no test files that referencelynx.requireModuleAsync(only the two setup files themselves do). Since no existing tests in this package invoke that method, throwing inrstest.jswon’t break anything. No further action needed.packages/react/testing-library/src/__tests__/lazy-bundle/index.test.jsx (3)
17-17: LGTM! Proper conditional rendering for rstest compatibility.The conditional rendering logic correctly handles the rstest environment where external component loading is not supported. The
typeofcheck prevents potential ReferenceError issues.
48-67: LGTM! Consistent test assertions for both environments.The branching test logic properly handles the different expected outcomes for rstest vs vitest environments. The snapshots correctly reflect the conditional rendering behavior.
17-17: No additional rstest conditionals needed in other testsA search across
packages/react/testing-library/src/__tests__forlazy(,import(, and__RSTEST__shows that onlylazy-bundle/index.test.jsxuses both external lazy imports and the__RSTEST__guard. There are no other test files requiring this pattern, so the existing conditional is appropriately scoped and consistent.packages/react/package.json (1)
97-98: New setup file exports enable direct access to testing configurations.The addition of direct exports for setup files is a good architectural decision that allows users to access environment-specific configurations without going through the main testing library exports.
packages/react/testing-library/src/plugins/rstest.ts (1)
3-24: Plugin configuration correctly enables HMR for rstest testing environment.The implementation properly configures HMR and adds the necessary webpack plugin for the testing environment. The logic ensures a
MIXEDbundle is generated by enabling HMR in development mode.packages/react/testing-library/rslib.config.ts (3)
8-8: Good: TypeScript declarations enabled for better type support.Enabling
dts: trueensures proper TypeScript declaration files are generated for the library consumers.
14-14: New plugins entry point aligns with the export structure changes.The addition of
'plugins/index': './src/plugins/index.ts'correctly supports the new plugin-based architecture and matches the export changes in package.json.
33-33: Glob pattern usage improves maintainability for setup and environment files.The updated source entry using glob patterns
['./src/index.jsx', './src/env/*', './src/setupFiles/**/*.js']is more maintainable and automatically includes new environment and setup files.packages/react/testing-library/src/setupFiles/common.js (2)
89-135: Environment-specific lynx.requireModuleAsync handling verifiedBoth environment-specific setup files explicitly define
target.lynx.requireModuleAsync, so the removal from the common setup is correctly scoped:
- packages/react/testing-library/src/setupFiles/vitest.js
Definestarget.lynx.requireModuleAsyncusing__vite_ssr_dynamic_import__for dynamic imports.- packages/react/testing-library/src/setupFiles/rstest.js
Definestarget.lynx.requireModuleAsyncto throw a “not implemented” error as a clear fallback.No further changes are needed here.
3-18: Incorrect import paths in common setup – modules not foundIt looks like the imports in
packages/react/testing-library/src/setupFiles/common.js (lines 3–18)
are pointing to files that don’t exist in the repo. I couldn’t locate any
runtime/liborworklet-runtime/libdirectories, nor the following files:• backgroundSnapshot.js
• snapshot.js
• lynxApi.jsPlease verify where these modules now live (or add them) and update the relative import paths accordingly.
packages/react/testing-library/vitest.config.ts (2)
1-11: Excellent simplification to plugin-based configuration.The new approach is much cleaner and more maintainable than the previous async config creation and merging. The plugin-based architecture is consistent with the overall refactoring.
6-8: Plugin configuration correctly maintains runtime package specification.The plugin configuration properly specifies the
runtimePkgNameas@lynx-js/react, maintaining compatibility with the existing testing setup.packages/rspeedy/plugin-react/src/loaders.ts (1)
10-64: LGTM! Well-structured testing environment loader setup.The
applyTestingEnvLoadersfunction follows the established pattern fromapplyLoadersand correctly:
- Extracts existing Rsbuild loaders (builtin:swc-loader, swc-loader)
- Applies them to the testing rule with appropriate options
- Clears default loaders to prevent JSX transformation conflicts
- Handles source map configuration consistently
packages/webpack/react-webpack-plugin/src/loaders/testing.ts (1)
16-119: LGTM! Robust testing loader with excellent error handling.The testing loader implementation is well-designed with:
- Proper handling of both rspack (
emitDiagnostic) and webpack APIs- Robust error handling for UTF-8 issues that could cause crashes
- Appropriate test-specific configuration (disabled DCE, shaking, refresh, etc.)
- Correct path normalization using the testing-library as reference point
The fallback error handling in the catch blocks (lines 76-80, 106-110) is particularly valuable for preventing crashes with invalid UTF-8 line/column data.
packages/react/testing-library/rstest.config.ts (1)
1-53: LGTM! Comprehensive rstest configuration with excellent compatibility approach.The configuration is well-structured with:
- Smart vitest polyfill aliasing for test case compatibility between frameworks
- Proper SWC configuration adjustment for class field consistency with vitest snapshots
- Complete environment setup with jsdom and required setup files
- Appropriate define flags and globals configuration
The approach of using a polyfill to maintain compatibility between vitest and rstest is particularly elegant and practical.
packages/rspeedy/plugin-react/src/entry.ts (2)
69-250: LGTM! Proper conditional entry processing for test environment.The conditional logic correctly skips complex entry point processing during testing, which is appropriate since testing environments don't need the full bundler chain setup with main-thread/background entries, hot module replacement, and template plugins.
The implementation correctly preserves access to
finalFirstScreenSyncTimingfor the ReactWebpackPlugin configuration outside the conditional.
66-67: No action needed for finalFirstScreenSyncTiming initialization.The initial assignment
let finalFirstScreenSyncTiming = firstScreenSyncTimingensures that even when the
if (process.env.NODE_ENV !== 'test')block is skipped,finalFirstScreenSyncTimingretains a valid fallback value. No further defaulting is required.packages/react/testing-library/src/plugins/vitest.ts (7)
1-12: Imports and module setup look good!The copyright header is properly included, and the ESM module setup with
createRequireis correctly implemented for compatibility with CommonJS modules.
14-21: Well-structured plugin options interface!The interface is properly documented with JSDoc, and the optional parameter with a clear default value provides good extensibility.
23-53: Solid package resolution and alias generation logic!The plugin correctly resolves package directories and efficiently handles alias generation, avoiding duplicates when the runtime package matches the OSS package.
54-59: Plugin configuration and initialization are properly set up!The 'pre' enforcement and async buildStart hook for package validation follow Vite plugin best practices.
128-140: Test configuration is comprehensive and well-structured!The config hook properly sets up the Vitest environment with custom environment, globals, setup files, and module aliases in the correct precedence order.
143-150: Helper functions are well-implemented!The
ensurePackagesInstalledfunction appropriately handles missing dependencies, andnormalizeSlashesensures cross-platform path compatibility.Also applies to: 167-169
60-127: Transform implementation is well-structured with one concernThe transform logic properly handles file filtering, path normalization, and error reporting. However, there's a potential runtime error with the non-null assertion.
return { code: result.code, - map: result.map!, + map: result.map ?? undefined, };The non-null assertion on
result.map!could cause a runtime error if the transform doesn't return a source map. Use nullish coalescing to safely handle this case.⛔ Skipped due to learnings
Learnt from: PupilTong PR: lynx-family/lynx-stack#1292 File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151 Timestamp: 2025-07-15T10:00:56.154Z Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.Learnt from: PupilTong PR: lynx-family/lynx-stack#1029 File: packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts:214-217 Timestamp: 2025-07-16T06:25:41.024Z Learning: In the lynx-stack codebase, CSS strings produced by `genCssContent` are considered trusted developer input, so additional sanitization/escaping is not required.Learnt from: PupilTong PR: lynx-family/lynx-stack#1029 File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99 Timestamp: 2025-07-16T06:28:26.421Z Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/testing-library/testing-environment/src/index.ts (1)
237-240: Consider adding cleanup mechanism for console augmentation.The direct console property assignment approach is simpler than the previous
createPreconfiguredConsolefunction, but there's no cleanup mechanism to restore the original console state. This could potentially cause issues if the testing environment is reset or reused.Consider implementing a cleanup mechanism in the
reset()method to restore the original console state if needed for future extensibility.Also applies to: 345-348
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/test.yml(1 hunks)package.json(1 hunks)packages/react/package.json(1 hunks)packages/react/testing-library/src/__tests__/alog.test.jsx(2 hunks)packages/react/testing-library/src/__tests__/list.test.jsx(15 hunks)packages/rspeedy/plugin-react/src/entry.ts(1 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(2 hunks)packages/testing-library/testing-environment/src/index.ts(2 hunks)packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
- .github/workflows/test.yml
- packages/react/package.json
- packages/rspeedy/plugin-react/src/entry.ts
- packages/react/testing-library/src/tests/list.test.jsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/react/testing-library/src/__tests__/alog.test.jsx (1)
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.177Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
🔇 Additional comments (2)
packages/react/testing-library/src/__tests__/alog.test.jsx (2)
45-76: Verify that the updated snapshots maintain correct test coverage.The inline snapshots have been updated with new unique IDs but maintain the same structure. The test logic correctly validates both main thread and background thread logging behavior.
The test assertions properly verify the dual-threaded logging behavior and the updated snapshots reflect the expected output structure.
Also applies to: 92-105
78-79: LGTM! Array clearing approach is appropriate.The manual array clearing approach (
mainThreadALogCalls = []) is appropriate for the custom override implementation and provides clear separation between test phases.Also applies to: 85-86
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
CodSpeed Performance ReportMerging #1305 will not alter performanceComparing Summary
|
Web Explorer#4403 Bundle Size — 366.68KiB (0%).ff2cb92(current) vs f66e319 main#4399(baseline) Bundle metrics
|
| Current #4403 |
Baseline #4399 |
|
|---|---|---|
 Initial JS |
143.49KiB |
143.49KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
7 |
7 |
| Assets |
7 |
7 |
 Modules |
210(-0.47%) |
211 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.89% |
3.89% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #4403 |
Baseline #4399 |
|
|---|---|---|
| JS |
234.68KiB |
234.68KiB |
| Other |
100.16KiB |
100.16KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch upupming:feat/rltl-rstest Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#4410 Bundle Size — 237.07KiB (0%).ff2cb92(current) vs f66e319 main#4406(baseline) Bundle metrics
|
| Current #4410 |
Baseline #4406 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
158 |
158 |
| Duplicate Modules |
64 |
64 |
| Duplicate Code |
45.86% |
45.86% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #4410 |
Baseline #4406 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.31KiB |
91.31KiB |
Bundle analysis report Branch upupming:feat/rltl-rstest Project dashboard
Generated by RelativeCI Documentation Report issue
…oid breaking current tests
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/test.yml(1 hunks)packages/react/testing-library/README.md(1 hunks)packages/react/testing-library/rstest.config.ts(1 hunks)packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js(1 hunks)packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/vitest.config.ts(1 hunks)packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.md(1 hunks)packages/rspeedy/plugin-react/src/entry.ts(2 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(4 hunks)packages/testing-library/examples/basic/rstest.config.ts(1 hunks)packages/use-sync-external-store/vitest.config.ts(1 hunks)packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md(1 hunks)packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/react/testing-library/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- .github/workflows/test.yml
- packages/react/testing-library/rstest.config.ts
- packages/rspeedy/plugin-react/src/entry.ts
- packages/testing-library/examples/basic/rstest.config.ts
- packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.421Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
🔇 Additional comments (7)
packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.md (1)
70-70: Add TSDoc for the newly-exposedenableTestingLibraryflagBecause this flag is now part of the public API surface, a short TSDoc description should be added in the source
PluginReactLynxOptionsdeclaration. Otherwise consumers will get no IntelliSense hint and the API report will continue to show the “(undocumented)” marker in future releases.
Example:/** * When true, swaps the standard RSBuild loaders for testing-oriented loaders * (e.g. JSDOM, React Testing Library, rstest) and skips HTML entry/template * injection. Defaults to `false`. */ enableTestingLibrary?: boolean;This also helps downstream maintainers understand the implicit default (
false) and any side-effects (e.g., entry/template skipping).
[ suggest_nitpick ]packages/use-sync-external-store/vitest.config.ts (1)
1-11: LGTM! Clean simplification of Vitest configuration.The refactor successfully removes unnecessary async complexity while maintaining all essential functionality. The plugin-based approach with
vitestTestingLibraryPluginis cleaner and more maintainable than the previous async config merging pattern.packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/vitest.config.ts (1)
1-6: LGTM! Template configuration properly simplified.The template configuration follows the same clean pattern as other files in the PR. The minimal configuration with just the plugin is appropriate for a template, allowing users to add their specific configuration as needed.
packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js (1)
1-6: LGTM! JavaScript template configuration properly simplified.The JavaScript template mirrors the TypeScript version appropriately, maintaining consistency across template variants while providing the same clean plugin-based configuration.
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts (3)
68-71: LGTM! Interface extension follows established patterns.The new
enableTestingLibraryproperty is properly typed and documented, maintaining consistency with the existing interface structure.
122-125: LGTM! Loader addition follows established conventions.The 'TESTING' loader is properly integrated into the existing loader structure with correct typing and path resolution.
146-146: LGTM! Sensible default value for testing library support.Setting
enableTestingLibrarytofalseby default is appropriate for opt-in testing library integration.
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
Outdated
Show resolved
Hide resolved
…vitest and rstest
…o feat/rltl-rstest
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/react/testing-library/src/setupFiles/common/runtime-setup.js (1)
1-174: Add missing package exports and implement rstest dynamic importsThe refactoring has placed the new setup scripts correctly, but two issues remain:
- package.json exports is empty
packages/react/testing-library/package.jsonshows"exports": null. You need to expose the new setup files so consumers can import them. For example:{ "exports": { "./setupFiles/vitest.js": "./src/setupFiles/vitest.js", "./setupFiles/rstest.js": "./src/setupFiles/rstest.js", "./setupFiles/common/bootstrap.js": "./src/setupFiles/common/bootstrap.js" } }- rstest requireModuleAsync is unimplemented
Inpackages/react/testing-library/src/setupFiles/inner/rstest.js,If rstest should support dynamic imports like Vitest, implement this method (e.g., usingtarget.lynx.requireModuleAsync = async (url, callback) => { throw new Error('lynx.requireModuleAsync not implemented for rstest'); };__vite_ssr_dynamic_import__or equivalent) or document why it’s intentionally unsupported.Please update these so the new setup files are fully consumable and functional.
🧹 Nitpick comments (3)
packages/react/testing-library/src/setupFiles/inner/rstest.js (1)
18-20: Consider documenting the limitation of requireModuleAsync stub.The stub implementation that throws an error is appropriate for rstest, but consider adding a comment explaining why this functionality is not available or if there are plans to implement it in the future.
+ // rstest doesn't support dynamic module loading in the same way as vitest target.lynx.requireModuleAsync = async (url, callback) => { throw new Error('lynx.requireModuleAsync not implemented for rstest'); };packages/react/testing-library/src/env/vitest.ts (1)
8-11: Consider adding error handling for jsdom setup.The jsdom setup could potentially fail. Consider wrapping it in a try-catch block to provide better error messages during test environment initialization.
+ try { const fakeGlobal: { jsdom?: any; } = {}; await builtinEnvironments.jsdom.setup(fakeGlobal, {}); global.jsdom = fakeGlobal.jsdom; + } catch (error) { + throw new Error(`Failed to setup jsdom environment: ${error.message}`); + }packages/react/testing-library/src/setupFiles/inner/vitest.js (1)
10-16: Consider adding type safety for the callback parameter.While the current implementation works well, consider adding basic validation for the callback parameter to prevent runtime errors if it's not a function.
target.lynx.requireModuleAsync = async (url, callback) => { + if (typeof callback !== 'function') { + throw new Error('callback must be a function'); + } try { callback(null, await __vite_ssr_dynamic_import__(url)); } catch (err) { callback(err, null); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/test.yml(2 hunks)packages/react/testing-library/rslib.config.ts(2 hunks)packages/react/testing-library/src/__tests__/lazy-bundle/index.test.jsx(3 hunks)packages/react/testing-library/src/env/index.ts(1 hunks)packages/react/testing-library/src/env/rstest.ts(1 hunks)packages/react/testing-library/src/env/vitest.ts(1 hunks)packages/react/testing-library/src/plugins/rstest.ts(1 hunks)packages/react/testing-library/src/setupFiles/common/bootstrap.js(1 hunks)packages/react/testing-library/src/setupFiles/common/runtime-setup.js(1 hunks)packages/react/testing-library/src/setupFiles/inner/rstest.js(1 hunks)packages/react/testing-library/src/setupFiles/inner/vitest.js(1 hunks)packages/react/testing-library/src/setupFiles/rstest.js(1 hunks)packages/react/testing-library/src/setupFiles/vitest.js(1 hunks)packages/react/testing-library/vitest.config.ts(1 hunks)packages/testing-library/testing-environment/src/index.ts(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/react/testing-library/src/env/index.ts
- packages/react/testing-library/src/env/rstest.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/react/testing-library/src/setupFiles/vitest.js
- packages/react/testing-library/vitest.config.ts
- packages/testing-library/testing-environment/src/index.ts
- packages/react/testing-library/src/plugins/rstest.ts
- .github/workflows/test.yml
- packages/react/testing-library/rslib.config.ts
- packages/react/testing-library/src/tests/lazy-bundle/index.test.jsx
🧰 Additional context used
🧠 Learnings (4)
packages/react/testing-library/src/setupFiles/rstest.js (1)
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.263Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/testing-library/src/setupFiles/common/runtime-setup.js (1)
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.263Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/testing-library/src/setupFiles/inner/rstest.js (1)
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.177Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
packages/react/testing-library/src/setupFiles/common/bootstrap.js (1)
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.263Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
🧬 Code Graph Analysis (3)
packages/react/testing-library/src/setupFiles/inner/vitest.js (2)
packages/react/testing-library/src/setupFiles/inner/rstest.js (1)
globalThis(9-11)packages/react/testing-library/src/setupFiles/common/runtime-setup.js (1)
globalThis(20-27)
packages/react/testing-library/src/setupFiles/inner/rstest.js (2)
packages/react/testing-library/src/setupFiles/inner/vitest.js (1)
globalThis(1-3)packages/react/testing-library/src/setupFiles/common/runtime-setup.js (1)
globalThis(20-27)
packages/react/testing-library/src/setupFiles/common/bootstrap.js (3)
packages/react/testing-library/src/setupFiles/inner/rstest.js (1)
globalThis(9-11)packages/react/testing-library/src/setupFiles/inner/vitest.js (1)
globalThis(1-3)packages/react/testing-library/src/setupFiles/common/runtime-setup.js (1)
globalThis(20-27)
🔇 Additional comments (7)
packages/react/testing-library/src/setupFiles/inner/rstest.js (1)
1-7: LGTM! Clean environment setup for rstest.The setup correctly initializes the testing environment by creating the LynxTestingEnv instance and establishing the global jsdom reference. The approach mirrors the vitest setup while being tailored for rstest.
packages/react/testing-library/src/env/vitest.ts (1)
4-24: Well-structured custom Vitest environment implementation.The environment correctly:
- Uses proper TypeScript types from vitest/environments
- Isolates jsdom setup using a fake global object
- Transfers necessary properties to the actual global
- Includes proper cleanup in teardown
packages/react/testing-library/src/setupFiles/common/bootstrap.js (1)
2-7: Add validation for required globals before injection.The code assumes that
lynxTestingEnvand its thread globals exist, but doesn't validate this. Consider adding checks to prevent runtime errors.+ if (!globalThis.lynxTestingEnv) { + throw new Error('lynxTestingEnv not initialized'); + } + + if (!globalThis.onInjectMainThreadGlobals || !globalThis.onInjectBackgroundThreadGlobals) { + throw new Error('Required injection functions not available'); + } + globalThis.onInjectMainThreadGlobals( globalThis.lynxTestingEnv.mainThread.globalThis, ); globalThis.onInjectBackgroundThreadGlobals( globalThis.lynxTestingEnv.backgroundThread.globalThis, );⛔ Skipped due to learnings
Learnt from: PupilTong PR: lynx-family/lynx-stack#1292 File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151 Timestamp: 2025-07-15T10:00:56.154Z Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.packages/react/testing-library/src/setupFiles/rstest.js (1)
1-4: LGTM! Well-structured setup orchestration for rstest.The import sequence is logical and follows the expected order:
- Environment setup
- Common runtime setup
- Framework-specific inner setup
- Bootstrap initialization
This provides a clean and maintainable setup pattern for rstest testing.
packages/react/testing-library/src/setupFiles/inner/vitest.js (1)
1-17: LGTM! Well-implemented vitest-specific background thread setup.The implementation correctly:
- Preserves existing
onInjectBackgroundThreadGlobalsfunctionality- Provides proper
requireModuleAsyncimplementation using vitest's dynamic import- Uses appropriate error handling with try-catch
- Follows Node.js callback convention with (error, result) parameters
This provides a clean contrast to the rstest version which appropriately stubs the functionality.
packages/react/testing-library/src/setupFiles/common/runtime-setup.js (2)
3-18: LGTM! Import paths updated correctly for directory restructuring.The import paths have been properly updated from
../../runtime/...to../../../../runtime/..., reflecting the new directory structure. This is consistent with the refactoring to support multiple testing frameworks.
1-174: Excellent refactoring to support multiple testing frameworks.The removal of framework-specific code (like
requireModuleAsyncimplementation and direct global injection calls) and the focus on common runtime setup functionality creates a cleaner separation of concerns. This approach allows framework-specific implementations to be handled in their respective setup files while maintaining shared core functionality.
packages/react/testing-library/src/setupFiles/common/bootstrap.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/App.jsx (2)
16-19: Consider removing the unused string literalThe string literal
'background only'appears to be unused and may be leftover from development or debugging.const onTap = useCallback(() => { - 'background only' setAlterLogo(!alterLogo) }, [alterLogo])
44-44: Minor inconsistency in file extension referenceThe hint text references
src/App.tsxbut this is a.jsxfile. This appears to be copied from a TypeScript template.- {' src/App.tsx '} + {' src/App.jsx '}packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/App.tsx (1)
18-21: Clarify the 'background only' commentThe string literal
'background only'on line 19 appears to be a comment or directive that's unclear in context. This might be specific to the Lynx ecosystem, but it would benefit from clarification.Consider adding a comment to explain the purpose of this string literal:
const onTap = useCallback(() => { - 'background only' + 'background only' // Lynx-specific directive for event handling setAlterLogo(!alterLogo) }, [alterLogo])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/lynx.config.js(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/package.json(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/rstest.config.js(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/App.jsx(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/__tests__/index.test.jsx(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/index.jsx(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/lynx.config.ts(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/package.json(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/rstest.config.ts(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/App.tsx(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/__tests__/index.test.tsx(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/index.tsx(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/rspeedy-env.d.ts(1 hunks)packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/rspeedy-env.d.ts
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/index.tsx
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/lynx.config.ts
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/rstest.config.ts
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/lynx.config.js
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/index.jsx
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/rstest.config.js
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/package.json
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/tsconfig.json
- packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/package.json
🧰 Additional context used
🧠 Learnings (1)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/__tests__/index.test.tsx (1)
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.263Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
🧬 Code Graph Analysis (3)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/App.jsx (3)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/App.tsx (1)
App(8-55)packages/web-platform/web-tests/tests/react/basic-mts-mainthread-nested-ref/index.jsx (1)
props(7-7)packages/react/transform/src/wasm.js (1)
view(24-28)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/App.tsx (1)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/App.jsx (3)
App(8-53)alterLogo(9-9)onTap(16-19)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/__tests__/index.test.jsx (2)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/App.jsx (1)
App(8-53)packages/react/runtime/__test__/utils/nativeMethod.ts (1)
elementTree(25-379)
🔇 Additional comments (18)
packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/__tests__/index.test.jsx (4)
1-9: LGTM: Proper test setup and importsThe test file correctly imports the necessary testing utilities from rstest and ReactLynx testing library. The imports are well-structured and follow the expected patterns for the new rstest integration.
10-27: LGTM: Comprehensive callback testingThe test properly verifies the
onMountedcallback behavior using rstest's mock function. The assertion checks both the call count and the specific arguments passed, including the__MAIN_THREAD__global variable state.
28-89: LGTM: Thorough DOM structure verificationThe inline snapshot effectively captures the complete rendered element tree, providing comprehensive regression testing. The snapshot correctly reflects the App component's structure with all CSS classes and attributes.
90-102: LGTM: Proper element querying and assertionsThe test correctly uses
getQueriesForElementto query the rendered component and verify specific elements. The combination offindByTextwithtoBeInTheDocumentand snapshot assertions provides robust testing coverage.packages/rspeedy/create-rspeedy/template-react-rstest-rltl-js/src/App.jsx (3)
1-7: LGTM: Proper imports and asset handlingThe imports are correctly structured, importing React hooks from the lynx-js package and including necessary CSS and image assets.
8-14: LGTM: Proper component structure and lifecycleThe component correctly uses
useStatefor logo state management anduseEffectfor mount behavior. The optional callback invocation follows React best practices.
21-53: LGTM: Well-structured JSX with proper ReactLynx elementsThe JSX structure properly uses ReactLynx's custom elements (
view,text,image) and follows good practices for layout and styling. The conditional rendering for logo toggling is implemented correctly.packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/App.tsx (3)
1-1: LGTM: Correct import for Lynx ecosystemThe import correctly uses
@lynx-js/reacthooks instead of standard React, which is appropriate for the Lynx testing template.
8-10: LGTM: Proper TypeScript interface definitionThe props interface correctly defines the optional
onMountedcallback with proper TypeScript typing.
28-32: LGTM: Correct Lynx event bindingThe
bindtapevent binding is correct for the Lynx ecosystem and properly toggles between the two logos based on state.packages/rspeedy/create-rspeedy/template-react-rstest-rltl-ts/src/__tests__/index.test.tsx (8)
5-6: LGTM: Correct rstest imports and testing library integrationThe imports correctly use
@rstest/corefor the testing framework and@lynx-js/react/testing-libraryfor the React testing utilities, which aligns with the PR objectives.
11-11: LGTM: Proper rstest mock function usageUsing
rstest.fn()for creating mock functions is the correct approach for the rstest framework.
21-27: LGTM: Inline snapshot assertion for callbackThe inline snapshot correctly captures the mock function calls and verifies the
__MAIN_THREAD__value isfalse.
28-89: LGTM: Comprehensive DOM structure snapshotThe inline snapshot thoroughly captures the rendered component structure with:
- Correct Lynx custom elements (view, image, text)
- Proper CSS classes and inline styles
- Accurate asset paths
- Complete component hierarchy
This provides robust regression testing for the component's output.
90-94: LGTM: Proper testing library queries usageThe test correctly uses
getQueriesForElementto get queries for the rendered element and properly usesfindByTextto locate specific content.
95-101: LGTM: Element-specific snapshot assertionThe inline snapshot for the specific element provides focused testing of the individual text element with proper attributes.
16-16: MAIN_THREAD is injected by the RSTest setupThe RSTest configuration for this template already includes the Testing Library setup file, which defines
__MAIN_THREAD__(along with the other Lynx globals) before any tests run:
- In
rstest.config.ts,setupFiles: [ require.resolve('@lynx-js/react/testing-library/setupFiles/rstest') ]ensures__MAIN_THREAD__is available.- Under the hood, the testing‐environment module in
@lynx-js/react/testing-librarysetsglobalThis.__MAIN_THREAD__.No further action is required here.
28-28: VerifyelementTreeinjection in your rstest setupI confirmed that your project’s
rstest.config.tsincludes:
testEnvironment: 'jsdom'setupFiles: [ require.resolve('@lynx-js/react/testing-library/setupFiles/rstest') ]Type declarations for
elementTreeexist in
• packages/react/testing-library/types/index.d.ts (declaresglobal.elementTree: ElementTree)However, I couldn’t locate the actual
setupFiles/rstestsource underpackages/react/testing-library. Please manually verify that:
require.resolve('@lynx-js/react/testing-library/setupFiles/rstest')resolves to a file that injectselementTreeontoglobalThis.- That setup module indeed calls the environment’s element-tree initializer (e.g.,
initElementTree()) and assigns it toglobal.elementTree.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
upupming
force-pushed
the
feat/rltl-rstest
branch
from
efab608 to
f0e1e5c
Compare
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
Outdated
Show resolved
Hide resolved
upupming
force-pushed
the
feat/rltl-rstest
branch
from
f07c04d to
8863f55
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rspeedy/plugin-react/test/config.test.ts (1)
2141-2141: Consider if async describe block is necessary.The describe block is marked as async, but it's unusual for describe blocks to be async since they typically just group tests rather than perform async operations. The async import inside could be moved to a beforeAll hook if needed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rspeedy/plugin-react/src/pluginReactLynx.ts(2 hunks)packages/rspeedy/plugin-react/src/rstest.ts(1 hunks)packages/rspeedy/plugin-react/src/stubRspeedyApi.ts(1 hunks)packages/rspeedy/plugin-react/test/config.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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#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: the component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork ...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Applied to files:
packages/rspeedy/plugin-react/src/rstest.tspackages/rspeedy/plugin-react/src/stubRspeedyApi.ts
📚 Learning: in the lynx-stack codebase, css selectors in ssr hydration are generated by their own packages, ensu...
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Applied to files:
packages/rspeedy/plugin-react/src/rstest.ts
🔇 Additional comments (3)
packages/rspeedy/plugin-react/src/rstest.ts (1)
6-15: LGTM! Clean and focused implementation.The function correctly configures externals to allow Node.js internal modules in the testing environment. The regex pattern
/^node:/properly matches Node.js built-in modules using the modern protocol format.packages/rspeedy/plugin-react/src/stubRspeedyApi.ts (1)
8-24: Implementation looks good with one consideration.The stub API implementation is clean and provides appropriate fallbacks for each method. The debug method correctly handles both string and function inputs.
However, the
@ts-expect-errorcomment on line 17-18 suggests a type mismatch between Rsbuild and Rspeedy configs. Consider if this approximation is sufficient or if additional type casting/adaptation is needed.Based on past review comments, there was discussion about this stub API not being rstest-specific. The current placement seems reasonable, but verify if this aligns with the final architectural decision.
packages/rspeedy/plugin-react/test/config.test.ts (1)
2141-2207: Well-structured tests for rstest integration.The tests effectively validate the conditional behavior when
callerNameis'rstest':
- CSS rules correctly remain as rsbuild defaults
- JS loaders properly switch to include the testing loader from react-webpack-plugin
- Inline snapshots provide clear expectations for the loader configurations
This aligns well with the broader rstest integration changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/rspeedy/plugin-react/src/rspeedy-api.ts (2)
8-14: Consider removing redundant non-null assertion.The function uses both a non-null assertion operator (
!) and aninvariantcheck, which is redundant. The invariant should be sufficient for runtime safety.Apply this diff to remove the redundant non-null assertion:
export function getRspeedyAPI(api: RsbuildPluginAPI): ExposedAPI { - const rspeedyAPI = api.useExposed<ExposedAPI>( - Symbol.for('rspeedy.api'), - )! - invariant(rspeedyAPI, 'Should have rspeedy.api') + const rspeedyAPI = api.useExposed<ExposedAPI>( + Symbol.for('rspeedy.api'), + ) + invariant(rspeedyAPI, 'Failed to retrieve rspeedy.api - ensure the rspeedy plugin is properly initialized') return rspeedyAPI }
8-8: Add JSDoc documentation for better API clarity.The function would benefit from documentation explaining its purpose, parameters, and return value, especially since it's part of a plugin API.
Add JSDoc documentation above the function:
+/** + * Retrieves the exposed Rspeedy API from an RsbuildPluginAPI instance. + * @param api - The RsbuildPluginAPI instance to retrieve the exposed API from + * @returns The exposed Rspeedy API + * @throws Invariant error if the rspeedy.api is not available + */ export function getRspeedyAPI(api: RsbuildPluginAPI): ExposedAPI {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.github/workflows/test.yml(3 hunks)package.json(1 hunks)packages/react/package.json(1 hunks)packages/react/testing-library/package.json(1 hunks)packages/react/testing-library/rstest.config.ts(1 hunks)packages/react/testing-library/src/__tests__/lazy-bundle/index.test.jsx(3 hunks)packages/rspeedy/create-rspeedy/package.json(1 hunks)packages/rspeedy/create-rspeedy/src/index.ts(2 hunks)packages/rspeedy/plugin-react/package.json(1 hunks)packages/rspeedy/plugin-react/src/entry.ts(3 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(3 hunks)packages/rspeedy/plugin-react/src/rspeedy-api.ts(1 hunks)packages/testing-library/examples/basic/package.json(1 hunks)packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md(1 hunks)packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/react/package.json
- packages/react/testing-library/package.json
- packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- package.json
- packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md
- packages/react/testing-library/src/tests/lazy-bundle/index.test.jsx
- pnpm-workspace.yaml
- packages/rspeedy/create-rspeedy/package.json
- packages/rspeedy/create-rspeedy/src/index.ts
- packages/testing-library/examples/basic/package.json
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- .github/workflows/test.yml
- packages/rspeedy/plugin-react/src/entry.ts
- packages/react/testing-library/rstest.config.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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#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: in pnpm, configdependencies in pnpm-workspace.yaml (used for plugins like @pnpm/plugin-better-defaul...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1447
File: pnpm-workspace.yaml:20-22
Timestamp: 2025-08-06T08:25:15.392Z
Learning: In pnpm, configDependencies in pnpm-workspace.yaml (used for plugins like pnpm/plugin-better-defaults and pnpm/plugin-trusted-deps) do not appear in or modify pnpm-lock.yaml. These plugins are managed separately by pnpm itself as configuration-level dependencies, not as part of the workspace's dependency tree. Running pnpm install after adding configDependencies will not change the lockfile.
Applied to files:
packages/rspeedy/plugin-react/package.json
📚 Learning: in pnpm, configdependencies in pnpm-workspace.yaml (used for plugins like @pnpm/plugin-better-defaul...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1447
File: pnpm-workspace.yaml:20-22
Timestamp: 2025-08-06T08:25:15.392Z
Learning: In pnpm, configDependencies in pnpm-workspace.yaml (used for plugins like pnpm/plugin-better-defaults and pnpm/plugin-trusted-deps) do not appear in or modify pnpm-lock.yaml. These plugins are managed separately by pnpm itself, not as part of the workspace's dependency tree. This is different from the internal configDependencies field that can appear within pnpm-lock.yaml for tracking configuration file dependencies.
Applied to files:
packages/rspeedy/plugin-react/package.json
📚 Learning: in pnpm, configdependencies (used for plugins like @pnpm/plugin-better-defaults and @pnpm/plugin-tru...
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1447
File: pnpm-workspace.yaml:20-22
Timestamp: 2025-08-06T08:25:15.392Z
Learning: In pnpm, configDependencies (used for plugins like pnpm/plugin-better-defaults and pnpm/plugin-trusted-deps) do not appear in pnpm-lock.yaml and do not require lockfile updates. They are installed separately in pnpm's own cache/store as configuration artifacts, not as part of the workspace's dependency tree.
Applied to files:
packages/rspeedy/plugin-react/package.json
📚 Learning: in the lynx-family/lynx-stack repository, ci checks require changesets when files matching the patte...
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:
packages/rspeedy/plugin-react/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/rspeedy/plugin-react/package.json (1)
48-48: LGTM! Dependency addition aligns with code usage.The addition of
tiny-invariant^1.3.3 is appropriate and directly supports the newgetRspeedyAPIfunction inrspeedy-api.ts.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/react/package.json (1)
107-107: Do we need to export setupFiles/rstest, or can the plugin/config auto-register it?This adds another public export. If the Vite/Vitest plugin can inject the setup file automatically, we can avoid expanding the exports surface.
If we keep it, verify the source exists and is wired to build:
#!/bin/bash fd -t f packages/react/testing-library/src/setupFiles/rstest.js
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/workflows/test.yml(3 hunks)package.json(1 hunks)packages/react/package.json(1 hunks)packages/react/testing-library/src/__tests__/lazy-bundle/index.test.jsx(3 hunks)packages/react/testing-library/src/__tests__/lynx.test.jsx(1 hunks)packages/react/testing-library/src/__tests__/worklet.test.jsx(5 hunks)packages/rspeedy/create-rspeedy/package.json(1 hunks)packages/rspeedy/plugin-react/package.json(1 hunks)packages/rspeedy/plugin-react/test/config.test.ts(1 hunks)packages/testing-library/testing-environment/src/index.ts(3 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- package.json
- packages/react/testing-library/src/tests/lynx.test.jsx
- packages/rspeedy/plugin-react/package.json
- packages/rspeedy/create-rspeedy/package.json
- packages/react/testing-library/src/tests/lazy-bundle/index.test.jsx
- .github/workflows/test.yml
- packages/react/testing-library/src/tests/worklet.test.jsx
- packages/rspeedy/plugin-react/test/config.test.ts
🧰 Additional context used
🧠 Learnings (3)
📓 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/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: 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.
📚 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/package.json
📚 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/package.json
🔇 Additional comments (4)
pnpm-workspace.yaml (1)
42-45: Verification Complete: pnpm ≥9 and catalog usage confirmed
- Root
package.jsondeclarespnpm@10.14.0(≥9), so catalogs are supported.- Every occurrence of
@rstest/coreacross the repo uses"catalog:rstest".- No direct version pins of
@rstest/corewere found.All checks pass—no further action needed.
packages/react/package.json (2)
103-106: No plugins export inpackages/reactI’ve checked and there’s no
pluginsimplementation or corresponding export inpackages/react/testing-libraryor itspackage.json. The AI summary mentioning@lynx-js/react/testing-library/pluginsappears to be incorrect—no action is needed here.Likely an incorrect or invalid review comment.
103-106: Switch to plugin-based export and deprecatevitest-configReplace the legacy export in
packages/react/package.json:- "./testing-library/vitest-config": { - "types": "./testing-library/dist/vitest.config.d.ts", - "default": "./testing-library/dist/vitest.config.js" - }, + "./testing-library/plugins": { + "types": "./testing-library/dist/plugins/index.d.ts", + "default": "./testing-library/dist/plugins/index.js" + },• Ensure the package’s build emits
packages/react/testing-library/dist/plugins/index.jsandindex.d.ts.
• Verify that consuming projects correctly resolve the new./testing-library/pluginsentrypoint.
• Deprecate or remove any documentation references to the oldvitest-configpath.packages/testing-library/testing-environment/src/index.ts (1)
8-8: No action needed: Default import from ‘node:events’ is safe under current TS configThe project’s root tsconfig.json enables esModuleInterop (and thus allows synthetic default imports), and the testing-environment tsconfig extends it. The default import of EventEmitter will type-check and run correctly, so you can keep the existing
import EventEmitter from 'node:events';as is.
If you prefer named imports for consistency, you may still write
-import EventEmitter from 'node:events'; +import { EventEmitter } from 'node:events';but it’s purely stylistic rather than required.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/react/testing-library/src/plugins/vitest.ts (1)
57-57: Optional: use a more descriptive plugin name.Rename to reflect purpose and avoid potential confusion with other ReactLynx transform plugins.
- name: 'transformReactLynxPlugin', + name: 'vitestTestingLibraryPlugin',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react/testing-library/src/plugins/vitest.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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/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: 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.
📚 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/plugins/vitest.ts
📚 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/plugins/vitest.ts
🧬 Code Graph Analysis (1)
packages/react/testing-library/src/plugins/vitest.ts (2)
packages/react/testing-library/src/plugins/index.ts (1)
testingLibraryPlugin(1-1)packages/react/testing-library/loaders/jsx-loader.js (4)
runtimePkgName(9-9)sourcePath(10-10)relativePath(12-15)basename(11-11)
🔇 Additional comments (1)
packages/react/testing-library/src/plugins/vitest.ts (1)
155-167: Nice follow-up on earlier feedback: exports checks and regex escaping added.Good job adding a guard for missing exports and escaping regex metacharacters in names. This prevents both runtime crashes and invalid RegExp patterns.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/testing-library/examples/basic/package.json (1)
9-9: LGTM: script name now clear and concise, aligns with prior feedbackRenaming to a simple "rstest" target improves clarity and addresses the earlier nitpick about ambiguous ":rs" and unnecessary "run".
🧹 Nitpick comments (1)
packages/testing-library/examples/basic/package.json (1)
6-12: Optional: add an alias for discoverabilityConsider adding a "test:rstest" alias so users can discover it alongside "test" scripts.
Apply this diff:
"scripts": { "build": "rspeedy build", "dev": "rspeedy dev", "rstest": "rstest", + "test:rstest": "pnpm run rstest", "test": "vitest", "test:type": "vitest --typecheck.only" },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react/testing-library/src/plugins/vitest.ts(1 hunks)packages/testing-library/examples/basic/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react/testing-library/src/plugins/vitest.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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: colinaaa
PR: lynx-family/lynx-stack#1500
File: .changeset/curvy-dragons-appear.md:1-5
Timestamp: 2025-08-13T11:50:58.069Z
Learning: In the lynx-family/lynx-stack repository, all packages under packages/react/ should use "lynx-js/react" in changesets since they are all published together under the same package name, regardless of their individual package.json names. This includes packages/react/testing-library and other sub-packages.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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: upupming
PR: lynx-family/lynx-stack#1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.
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.530Z
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.
🔇 Additional comments (1)
packages/testing-library/examples/basic/package.json (1)
9-9: Rstest CLI resolution: OK — no change requiredRoot-level devDependency provides the rstest binary and CI runs the script via the workspace, so the example's "rstest" script will resolve.
- packages/testing-library/examples/basic/package.json — scripts.rstest: "rstest" (no local @rstest/core)
- package.json — devDependencies contains "@rstest/core": "catalog:rstest"
- .github/workflows/test.yml — runs "pnpm -r run rstest --testTimeout=50000"
- packages/testing-library/examples/basic/rstest.config.ts — present and references setupFiles (require.resolve('@lynx-js/react/testing-library/setupFiles/rstest'))
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react/testing-library/src/__tests__/css/index.test.jsx (1)
37-38: Strengthen assertion and use runner-agnostic matcher; fix grammar.
- The current check is very weak (any string passes) compared to the previous regex. Also,
toBeTypeOfis Vitest-specific; usingtypeofkeeps this compatible across runners (rstest bridge, Vitest).- Minor grammar: “an string” → “a string”.
Apply this diff:
- // to be an string - expect(style3.baz).toBeTypeOf('string'); + // should be a non-empty string; rstest may not generate deterministic hashes + expect(typeof style3.baz).toBe('string'); + expect(style3.baz).not.toHaveLength(0);Optional: If feasible in both environments, further assert shape to ensure it looks like inline CSS (e.g., contains a colon):
expect(style3.baz).toContain(':');Verification: Please confirm
toBeTypeOfis available across your rstest bridge. If not, the proposed change avoids reliance on it.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react/testing-library/src/__tests__/css/index.test.jsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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: colinaaa
PR: lynx-family/lynx-stack#1500
File: .changeset/curvy-dragons-appear.md:1-5
Timestamp: 2025-08-13T11:50:58.069Z
Learning: In the lynx-family/lynx-stack repository, all packages under packages/react/ should use "lynx-js/react" in changesets since they are all published together under the same package name, regardless of their individual package.json names. This includes packages/react/testing-library and other sub-packages.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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: upupming
PR: lynx-family/lynx-stack#1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.
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.530Z
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.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rspeedy/plugin-react/src/entry.ts (1)
300-340: Bug: getChunks pushes imports into itself for array entriesWhen entryValue contains a string[], the code does imports.push(...imports) instead of imports.push(...item). This will either no-op or duplicate existing imports, and never add the intended array entries. Also, item.import may be undefined for partial entry descriptions.
Apply this diff to fix both issues:
for (const item of entryValue) { if (typeof item === 'string') { imports.push(item) continue } if (Array.isArray(item)) { - imports.push(...imports) + // item is string[] + imports.push(...item) continue } - const { dependOn } = item - - if (Array.isArray(item.import)) { - imports.push(...item.import) - } else { - imports.push(item.import) - } + const { dependOn } = item + // item.import can be string | string[] | undefined on partial configs + const imp = (item as any).import + if (Array.isArray(imp)) { + imports.push(...imp) + } else if (typeof imp === 'string') { + imports.push(imp) + } if (!dependOn) { continue } if (typeof dependOn === 'string') { chunks.unshift(dependOn) } else { chunks.unshift(...dependOn) } }Optionally, consider deduplicating imports before returning to avoid duplicates from multiple sources:
return { chunks, imports: Array.from(new Set(imports)) }
♻️ Duplicate comments (1)
packages/react/package.json (1)
103-106: Ensure vitest.config.ts is built with accompanying .d.ts or correct the export mappingThe current rslib setup in packages/react/testing-library/rslib.config.ts only includes ./src/vitest.config.ts in the “index” entry with dts: false, so no standalone dist/vitest.config.js or dist/vitest.config.d.ts is emitted. As a result, your new export in packages/react/package.json:
“./testing-library/vitest-config”: {
“types”: “./testing-library/dist/vitest.config.d.ts”,
“default”: “./testing-library/dist/vitest.config.js”
}will point at non-existent files and break downstream imports.
Please address this by choosing one of:
• Add a dedicated lib entry for the Vitest config in rslib.config.ts (with dts enabled), for example:
lib: [ + { + format: 'esm', + syntax: 'es2022', + bundle: false, + dts: true, + source: { entry: { 'vitest.config': './src/vitest.config.ts' } }, + }, { format: 'esm', syntax: 'es2022', @@ ]This will emit both dist/vitest.config.js and dist/vitest.config.d.ts.
• Or adjust the export to point at an existing build artifact (e.g. dist/index.js and dist/index.d.ts), and rename the export key accordingly.
Either way, verify after build that the files exist and consumers can
import vitest-configwithout missing types.
🧹 Nitpick comments (7)
packages/react/testing-library/src/__tests__/list.test.jsx (3)
34-34: Snapshot identifier churn is expected; verify determinism across runsThe updated inline snapshots for update-list-info (type: "Card:_snapshot_d0c07_test*") are consistent with the new test environment/plugin and look correct. As a future improvement to reduce churn and flakiness, consider asserting the parsed JSON shape (positions, item-keys, flags) while ignoring dynamic type identifiers.
Example pattern (keep your existing full DOM snapshots if you still find them valuable for visual regression):
const uli = JSON.parse(list.getAttribute('update-list-info')); // Only assert what matters for behavior: expect( uli.map(({ insertAction = [], removeAction = [], updateAction = [] }) => ({ insertAction: insertAction.map(({ position, 'item-key': itemKey, 'full-span': fullSpan }) => ({ position, itemKey, fullSpan: !!fullSpan, })), removeAction, updateAction, // keep if you rely on it })), ).toEqual([ // ...expected structural actions here, without asserting "type" ]);Also applies to: 45-45, 64-64, 94-94, 124-124, 206-206, 398-481, 483-486, 602-602, 626-626, 638-638, 749-749, 807-807, 827-827, 846-846
483-569: Remove redundant duplicate assertionThis second expect(list).toMatchInlineSnapshot(...) block is identical to the previous one and runs immediately afterward without any intervening operations. It adds noise and increases test time without increasing coverage.
Apply this diff to remove the duplication:
- expect(list).toMatchInlineSnapshot(` - <list - style="width:100%;height:100%" - update-list-info="[{"insertAction":[{"position":0,"type":"__Card__:__snapshot_d0c07_test_7","item-key":"0","full-span":true},{"position":1,"type":"__Card__:__snapshot_d0c07_test_7","item-key":"1","full-span":true},{"position":2,"type":"__Card__:__snapshot_d0c07_test_7","item-key":"2","full-span":true},{"position":3,"type":"__Card__:__snapshot_d0c07_test_7","item-key":"3","full-span":true},{"position":4,"type":"__Card__:__snapshot_d0c07_test_7","item-key":"4","full-span":true},{"position":5,"type":"__Card__:__snapshot_d0c07_test_7","item-key":"5","full-span":true}],"removeAction":[],"updateAction":[]},{"insertAction":[],"removeAction":[3],"updateAction":[]}]" - > - <list-item - full-span="true" - item-key="4" - > - <wrapper> - <view> - <text> - 4 - </text> - <text> - 4 - </text> - </view> - <view> - <text> - hello - </text> - </view> - </wrapper> - </list-item> - <list-item - full-span="true" - item-key="5" - > - <wrapper> - <view> - <text> - 5 - </text> - <text> - 5 - </text> - </view> - <view> - <text> - hello - </text> - </view> - </wrapper> - </list-item> - <list-item - full-span="true" - item-key="2" - > - <wrapper> - <view> - <text> - 2 - </text> - <text> - 2 - </text> - </view> - <view> - <text> - hello - </text> - </view> - </wrapper> - </list-item> - <list-item - full-span="true" - item-key="1" - > - <wrapper> - <view> - <text> - 1 - </text> - <text> - 1 - </text> - </view> - <view> - <text> - hello - </text> - </view> - </wrapper> - </list-item> - </list> - `);
6-6: Optional: auto-restore/clear mocks to avoid cross-test leakageIf restoreMocks is not enabled in your test config, consider restoring and clearing spies after each test to keep counts/behavior isolated. This is especially relevant as you now use vi.spyOn in this file.
Apply this diff to import afterEach:
-import { describe, expect, vi } from 'vitest'; +import { afterEach, describe, expect, vi } from 'vitest';Then, add this near the top of the file (after imports):
afterEach(() => { vi.restoreAllMocks(); vi.clearAllMocks(); });packages/rspeedy/plugin-react/src/entry.ts (4)
83-87: Template filename resolution: OK; consider object-first resolution orderThe fallback to '[name].[platform].bundle' is sensible. Minor nit: when filename is an object, you prefer bundle over template. If template is the canonical target for HTML-like assets, consider inverting that priority; otherwise, this is fine as long as downstream expects 'bundle' first.
93-101: Intermediate path construction: avoid hard-coding defaultmain-thread asset placement uses DEFAULT_DIST_PATH_INTERMEDIATE for Lynx. There’s a TODO to “config intermediate”; see suggestion below to import the shared constant instead of redefining it locally to prevent drift.
262-265: Potential plugin ordering hazard when rstest skips LynxTemplatePluginYou call .after(PLUGIN_NAME_TEMPLATE) even when the rstest path doesn’t register LynxTemplatePlugin. Depending on the chain impl, this can be a no-op or reorder failure. Recommend avoiding .after when rstest is active or when the target plugin isn’t present.
If needed, adjust as:
- chain - .plugin(PLUGIN_NAME_REACT) - .after(PLUGIN_NAME_TEMPLATE) + const reactPlugin = chain.plugin(PLUGIN_NAME_REACT) + if (api.context.callerName !== 'rstest') { + reactPlugin.after(PLUGIN_NAME_TEMPLATE) + } - .use(ReactWebpackPlugin, [{ + reactPlugin.use(ReactWebpackPlugin, [{ // ... }])
30-33: Avoid duplicating DEFAULT_DIST_PATH_INTERMEDIATE; import the shared constantThis local constant duplicates the core export (packages/rspeedy/core/src/config/output/dist-path.ts). Prefer importing to prevent future drift.
Apply this within this file and the imports block:
-const DEFAULT_DIST_PATH_INTERMEDIATE = '.rspeedy' +// Prefer shared constant from core to prevent drift with tooling and other plugins +import { DEFAULT_DIST_PATH_INTERMEDIATE } from '@rspeedy/core/config/output/dist-path'If cross-package import paths differ in this repo, adjust the module path accordingly to point to the core package’s public export.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.github/workflows/test.yml(3 hunks)package.json(1 hunks)packages/react/package.json(1 hunks)packages/react/testing-library/package.json(1 hunks)packages/react/testing-library/src/__tests__/list.test.jsx(15 hunks)packages/rspeedy/create-rspeedy/package.json(1 hunks)packages/rspeedy/plugin-react/package.json(1 hunks)packages/rspeedy/plugin-react/src/entry.ts(3 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(3 hunks)packages/rspeedy/plugin-react/test/config.test.ts(1 hunks)packages/testing-library/examples/basic/package.json(1 hunks)packages/testing-library/testing-environment/src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- package.json
- packages/rspeedy/create-rspeedy/package.json
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- packages/react/testing-library/package.json
- packages/rspeedy/plugin-react/package.json
- packages/rspeedy/plugin-react/test/config.test.ts
- packages/testing-library/testing-environment/src/index.ts
- packages/testing-library/examples/basic/package.json
- .github/workflows/test.yml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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: upupming
PR: lynx-family/lynx-stack#1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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/package.jsonpackages/react/testing-library/src/__tests__/list.test.jsx
📚 Learning: 2025-08-11T05:59:28.530Z
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.530Z
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/package.jsonpackages/react/testing-library/src/__tests__/list.test.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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/package.json
📚 Learning: 2025-08-11T06:00:04.376Z
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.376Z
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/package.json
🧬 Code Graph Analysis (1)
packages/rspeedy/plugin-react/src/entry.ts (2)
packages/rspeedy/plugin-react/src/rspeedy-api.ts (1)
getRspeedyAPI(8-14)packages/rspeedy/core/src/config/output/dist-path.ts (1)
DEFAULT_DIST_PATH_INTERMEDIATE(29-29)
🔇 Additional comments (9)
packages/react/testing-library/src/__tests__/list.test.jsx (1)
6-6: Explicit vitest import looks good and aligns with rstest/vitest setupImporting vi alongside describe/expect is consistent with the new test runtime and enables spyOn usage in this file.
packages/rspeedy/plugin-react/src/entry.ts (8)
23-23: Good call: centralize config via getRspeedyAPIUsing the exposed Rspeedy API ensures a single source of truth for filename decisions and related config.
62-62: Initialize finalFirstScreenSyncTiming earlyStoring a local, mutable timing and updating it based on computed inlineScripts later is clear and safe.
168-201: Template plugin wiring looks correct for non-rstest
- Correctly pairs [main-thread, background] as chunks.
- Uses per-entry filename templating with [name] and [platform].
- Passes through the full set of Lynx options.
No issues spotted.
214-217: Synchronize firstScreen timing with inlineScriptsDowngrading to 'jsReady' when inline scripts aren’t used matches typical first-screen semantics. Good.
246-251: Web encode plugin is correctly gated to non-rstest web envAttaching WebEncodePlugin only for web (and not in rstest) avoids unnecessary encode passes in tests. LGTM.
254-261: Nice: logger fallback when chunkSplit disables extractStrSwitching to (api.logger ?? console).warn ensures visibility in all contexts and avoids silent behavior. LGTM.
277-287: Profile detection via original config: good improvementFetching the ‘original’ rsbuild config avoids using normalized/merged state and preserves user intent. LGTM.
68-71: Verified rstest gating: callerName usage is consistentAll occurrences of
api.context.callerName === 'rstest'line up with howcallerNameis set:• Entry plugin (packages/rspeedy/plugin-react/src/entry.ts: 68–71)
• Lynx plugin (packages/rspeedy/plugin-react/src/pluginReactLynx.ts: 375–378)
• Tests for React plugin (packages/rspeedy/plugin-react/test/config.test.ts: 2245–2246)
• Core API defaults to'rspeedy'and tests for custom callerName (packages/rspeedy/core/src/create-rspeedy.ts & core/test/createRspeedy.test.ts)No missing entry points or inconsistent callerName assignments were found.
4 participants
Summary
close #1300
In this PR, we added
rstestsupport for ReactLynx Testing Library. All tests of RLTL are passed successfully.User can use rstest with RLTL now:
The usage of
vitestalso exports the internal vite plugin for user customization, you can use it like:Checklist
Summary by CodeRabbit
New Features
Improvements
Chores