feat: support react compiler for ReactLynx #1269
Conversation
🦋 Changeset detectedLatest commit: a52d7b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 React Compiler support: plugin dedupe, Babel plugin wiring, example projects (standalone + testing-library), Vitest transformer and configs with an experimental enable flag, numerous tests exercising compiler optimizations, and related package manifests and assets. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
packages/rspeedy/plugin-react/package.json (1)
49-60:@rsbuild/plugin-babelmust be a runtime dependency, not dev-onlyThe loader code that this package exports calls into
@rsbuild/plugin-babelat build time in the consumer’s project.
Because npm does not installdevDependenciesof published packages for consumers, the module will be missing andrequire.resolve("@rsbuild/plugin-babel")will throw.Move the entry to
dependencies(or at leastpeerDependencies) and keep the pinned version to avoid accidental breakage."dependencies": { "@lynx-js/css-extract-webpack-plugin": "workspace:*", @@ "background-only": "workspace:^", + "@rsbuild/plugin-babel": "1.0.5" }, "devDependencies": { @@ - "@rsbuild/plugin-babel": "1.0.5",
🧹 Nitpick comments (3)
examples/react-compiler/src/rspeedy-env.d.ts (1)
1-2: File lacks trailing newlinePOSIX tools and many editors expect text files to end with a newline.
Add a blank line at the end to remove the warning./// <reference types="@lynx-js/rspeedy/client" /> +packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
24-32: Consider adding source map validation.The current implementation assumes
result?.code && result?.mapare sufficient checks, but you might want to validate that the source map is valid JSON.if (result?.code && result?.map) { - this.callback(null, result.code, JSON.stringify(result.map)); + try { + this.callback(null, result.code, JSON.stringify(result.map)); + } catch (error) { + this.callback(new Error(`Failed to serialize source map: ${error.message}`)); + } return; }packages/react/transform/src/lib.rs (1)
787-808: Consider adding input validation.The function assumes the input code is valid, but you might want to add basic validation for empty or invalid input.
fn is_react_compiler_required_inner(code: &str) -> bool { + if code.trim().is_empty() { + return false; + } + let cm = Lrc::new(SourceMap::default()); let fm = cm.new_source_file(FileName::Anon.into(), code.to_string());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
Cargo.lockis excluded by!**/*.lockexamples/react-compiler/src/assets/arrow.pngis excluded by!**/*.pngexamples/react-compiler/src/assets/lynx-logo.pngis excluded by!**/*.pngexamples/react-compiler/src/assets/react-logo.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
.changeset/plenty-baboons-tan.md(1 hunks).changeset/sour-weeks-wait.md(1 hunks)examples/react-compiler/eslint.config.mjs(1 hunks)examples/react-compiler/lynx.config.ts(1 hunks)examples/react-compiler/package.json(1 hunks)examples/react-compiler/src/App.css(1 hunks)examples/react-compiler/src/App.tsx(1 hunks)examples/react-compiler/src/index.tsx(1 hunks)examples/react-compiler/src/rspeedy-env.d.ts(1 hunks)examples/react-compiler/tsconfig.json(1 hunks)packages/react/transform/Cargo.toml(1 hunks)packages/react/transform/index.d.ts(1 hunks)packages/react/transform/src/lib.rs(4 hunks)packages/rspeedy/plugin-react/package.json(1 hunks)packages/rspeedy/plugin-react/src/loaders.ts(1 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(4 hunks)packages/webpack/react-webpack-plugin/package.json(1 hunks)packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts(1 hunks)
⏰ 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). (5)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: zizmor
🔇 Additional comments (19)
.changeset/plenty-baboons-tan.md (1)
1-6: No issues — documentation only
Nothing to change here..changeset/sour-weeks-wait.md (1)
1-16: No issues — documentation only
Looks good.packages/react/transform/Cargo.toml (1)
22-22: LGTM! Correct dependency update for React compiler support.The addition of the "swc_ecma_react_compiler" feature flag to the swc_core dependency is necessary to enable React compiler functionality in the Rust transformation layer.
examples/react-compiler/src/index.tsx (1)
1-11: LGTM! Clean React app entry point with proper HMR support.The implementation follows standard React patterns with appropriate use of the Lynx React package and webpack HMR integration for development.
packages/react/transform/index.d.ts (1)
621-622: LGTM! Well-designed API for React compiler requirement detection.The function declarations provide both async and sync variants with appropriate signatures. The AbortSignal support in the async version is a good addition for cancellation scenarios.
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts (1)
117-121: LGTM! Proper integration of React compiler loader.The React compiler loader is correctly registered in the plugin's loader system with appropriate type definitions.
examples/react-compiler/lynx.config.ts (1)
1-22: Well-structured configuration for React compiler integration.The configuration properly enables the experimental React compiler feature and includes appropriate supporting plugins. The conditional performance profiling based on environment variables is a good practice for development workflows.
examples/react-compiler/tsconfig.json (1)
1-19: Appropriate TypeScript configuration for Lynx React project.The configuration correctly sets up JSX compilation with
@lynx-js/reactas the import source and includes proper project references. The compiler options are suitable for the example project.examples/react-compiler/eslint.config.mjs (1)
1-19: Modern ESLint configuration with appropriate rule sets.The configuration uses the current flat config format and includes proper linting rules for JavaScript, TypeScript, and React hooks. The ignore pattern for the build directory is correctly configured.
packages/rspeedy/plugin-react/src/pluginReactLynx.ts (4)
30-30: Clean import addition for React compiler support.The import for
applyReactCompileris appropriately added to support the new experimental feature.
310-315: Well-documented experimental React compiler option.The new option is properly typed, documented with a link to React compiler documentation, and clearly marked as experimental. The JSDoc comment provides good context for users.
366-366: Appropriate default value for experimental feature.Setting the default value to
falsefor the experimental React compiler feature is the correct approach, ensuring users must explicitly opt-in to experimental functionality.
426-428: Proper conditional application of React compiler.The conditional logic correctly applies the React compiler only when the experimental flag is enabled, maintaining the plugin's existing patterns and ensuring the feature is opt-in.
examples/react-compiler/src/App.tsx (2)
1-14: Well-structured React component with proper hook usage.The component follows React best practices with appropriate imports, state management, and effect usage. The console logging on mount is appropriate for an example application.
20-52: Clean component rendering with proper Lynx elements.The component uses appropriate Lynx framework elements (
view,image,text) and follows the framework's patterns for event handling (bindtap) and styling. The conditional rendering and layout structure are well-implemented.examples/react-compiler/package.json (1)
21-21: Confirmed experimental React Compiler versions and compatibility notes.
- The example’s
package.jsonuses0.0.0-experimental-0566679-20250709for bothbabel-plugin-react-compilerandreact-compiler-runtime—this is the latest available experimental release.- React version support:
• Fully supported on React 19 RC.
• For React 17 or 18, you must includereact-compiler-runtimeand set the appropriatetargetin your Babel/Vite config.- Metro bundler (Expo/React Native) users: be aware of a known cyclic-dependency bug that can lead to undefined hooks at runtime.
- Avoid mismatched React/React-DOM versions or duplicate copies of React internals in dependencies—lock all React packages to the same version.
- Best practices:
• Runnpx react-compiler-healthcheckto validate your setup.
• Reserve this experimental compiler for demos/non-production or advanced testing until a stable GA release.Please verify that your project’s React version, build config, and dependency tree align with these requirements.
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
12-45: Well-implemented React Compiler loader.The loader correctly:
- Checks file extensions (.jsx, .tsx) before processing
- Uses the synchronous detection function to determine if transformation is needed
- Applies Babel transformation with appropriate plugins
- Handles success and error cases properly
- Falls back to original content when transformation isn't needed
packages/react/transform/src/lib.rs (1)
768-825: Solid implementation of React Compiler detection.The implementation correctly:
- Follows the existing Task pattern for async operations
- Uses appropriate TypeScript syntax configuration with decorators and TSX
- Handles parse errors gracefully by returning false
- Provides both sync and async variants
- Integrates properly with the N-API exports
examples/react-compiler/src/App.css (1)
1-120: Well-designed CSS with modern styling techniques.The stylesheet effectively creates a visually appealing interface with:
- Proper use of CSS variables for theming
- Responsive design with viewport units
- Smooth animations for interactive elements
- Good typography hierarchy
- Appropriate use of flexbox for layout
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/rspeedy/plugin-react/src/reactCompiler.ts (1)
18-27: Consider adding error handling for chain modification.The bundler chain modification looks correct, but there's no error handling if the JavaScript rule doesn't exist or if the loader fails to be added.
api.modifyBundlerChain((chain, { CHAIN_ID }) => { const rule = chain.module.rules.get(CHAIN_ID.RULE.JS) + + if (!rule) { + throw new Error('JavaScript rule not found in bundler chain') + } if (experimental_enableReactCompiler) { rule.use('react-compiler') .loader(ReactWebpackPlugin.loaders.REACT_COMPILER) .end() } })packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
22-51: Improve error handling and consider caching resolved packages.The dynamic package resolution logic is correct but could be optimized:
- The error handling for missing packages is good
- Consider caching resolved package paths to avoid repeated resolution
- The rootContext fallback is appropriate
+ // Consider adding a module-level cache for resolved packages + const packageCache = new Map<string, string>(); const [ babelPath, babelPluginReactCompilerPath, babelPluginSyntaxJsxPath, ] = [ '@babel/core', 'babel-plugin-react-compiler', '@babel/plugin-syntax-jsx', ].map((name) => { + const cached = packageCache.get(name); + if (cached) return cached; + try { - return require.resolve(name, { + const resolved = require.resolve(name, { paths: [this.rootContext], }); + packageCache.set(name, resolved); + return resolved; } catch { missingBabelPackages.push(name); } return ''; });
📜 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 (5)
examples/react-compiler/package.json(1 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(4 hunks)packages/rspeedy/plugin-react/src/reactCompiler.ts(1 hunks)packages/webpack/react-webpack-plugin/package.json(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/react-compiler/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/webpack/react-webpack-plugin/package.json
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
🔇 Additional comments (6)
packages/rspeedy/plugin-react/src/reactCompiler.ts (2)
1-9: LGTM - Clean imports and type declarations.The imports are well-structured and properly typed. The copyright header follows the project's convention.
10-17: LGTM - Function signature and destructuring are correct.The function signature uses proper TypeScript types and the destructuring pattern for extracting the experimental flag is clean.
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (4)
1-9: LGTM - Proper imports and type declarations.The imports are correctly structured, including the node:module import for createRequire and proper typing for the loader context.
10-19: LGTM - Function signature and setup are correct.The async loader function is properly typed and the dynamic import setup using createRequire is appropriate for this context.
20-21: LGTM - File extension filtering is correct.The regex pattern correctly targets JSX and TSX files for React compiler processing.
71-77: LGTM - Error handling and fallback are correct.The try-catch block properly handles errors and the fallback to return original content is appropriate.
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
packages/react/testing-library/src/vitest.config.js (1)
139-139: Update React Compiler target to match React 19.The project uses React 19.1.0, but the compiler is configured with target '17'.
- [babelPluginReactCompilerPath, { target: '17' }], + [babelPluginReactCompilerPath, { target: '19' }],packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (2)
69-75: Improve Babel transform result validation.Make the validation more robust and provide better error messages.
- if (result?.code && result?.map) { + if (result?.code != null && result?.map != null) { return callback(null, result.code, JSON.stringify(result.map)); } else { return callback( - new Error('babel-plugin-react-compiler transform failed'), + new Error(`babel-plugin-react-compiler transform failed for ${this.resourcePath}: ${result ? 'missing code or map' : 'no result'}`), ); }
61-61: Update React Compiler target to match React 19.The project uses React 19.1.0, but the compiler is configured with target '17'.
- [babelPluginReactCompilerPath!, { target: '17' }], + [babelPluginReactCompilerPath!, { target: '19' }],
🧹 Nitpick comments (3)
packages/react/testing-library/src/vitest.config.js (2)
84-84: Remove debug console.log statement.This appears to be debug code that should not be in production.
- console.log('reactCompilerRuntimeAlias', reactCompilerRuntimeAlias);
95-99: Consider removing the empty string return for CSS files.Returning an empty string for CSS files might cause issues. Consider returning null or undefined to let Vite handle them normally.
- return ''; + return null;packages/testing-library/examples/react-compiler/src/__tests__/expectLogs.jsx (1)
8-17: Consider using a more explicit array clearing approach.While
logs.length = 0works, usinglogs.splice(0)might be more explicit about the intent to clear the array.- logs.length = 0; + logs.splice(0);
📜 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 (16)
examples/react-compiler/package.json(1 hunks)examples/react-compiler/src/App.tsx(1 hunks)packages/react/testing-library/src/vitest.config.js(3 hunks)packages/react/testing-library/types/vitest-config.d.ts(1 hunks)packages/testing-library/examples/react-compiler/.gitignore(1 hunks)packages/testing-library/examples/react-compiler/lynx.config.ts(1 hunks)packages/testing-library/examples/react-compiler/package.json(1 hunks)packages/testing-library/examples/react-compiler/src/App.tsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/expectLogs.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/hello.test.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/index.test.tsx(1 hunks)packages/testing-library/examples/react-compiler/src/index.tsx(1 hunks)packages/testing-library/examples/react-compiler/src/rspeedy-env.d.ts(1 hunks)packages/testing-library/examples/react-compiler/tsconfig.json(1 hunks)packages/testing-library/examples/react-compiler/vitest.config.ts(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/testing-library/examples/react-compiler/src/rspeedy-env.d.ts
- packages/testing-library/examples/react-compiler/.gitignore
- packages/testing-library/examples/react-compiler/tsconfig.json
- packages/testing-library/examples/react-compiler/src/index.tsx
- packages/testing-library/examples/react-compiler/lynx.config.ts
- packages/testing-library/examples/react-compiler/vitest.config.ts
- packages/testing-library/examples/react-compiler/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/react-compiler/src/App.tsx
- examples/react-compiler/package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/testing-library/examples/react-compiler/src/App.tsx (1)
examples/react-compiler/src/App.tsx (1)
App(22-67)
🪛 GitHub Check: code-style-check
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
[failure] 63-63:
Type '[string | undefined, { isTSX: true; }] | null' is not assignable to type 'PluginItem'.
⏰ 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). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (10)
packages/react/testing-library/types/vitest-config.d.ts (2)
7-7: Good JSDoc correction!Using
@defaultValueinstead of@defaultaligns with TypeScript JSDoc standards.
10-17: Well-documented experimental feature flag.The property is properly typed, documented with a relevant link, and clearly marked as experimental with a sensible default.
packages/react/testing-library/src/vitest.config.js (1)
86-161: Well-structured React Compiler plugin implementation.The plugin correctly:
- Checks file extensions before processing
- Uses the transform API to detect compiler requirements
- Handles missing dependencies with clear error messages
- Provides proper source map support
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
10-82: Well-implemented React Compiler loader with proper async handling.The loader correctly:
- Uses webpack's async callback pattern
- Checks file extensions before processing
- Dynamically resolves dependencies from the project root
- Provides clear error messages for missing packages
- Handles TypeScript files with appropriate syntax plugin
packages/testing-library/examples/react-compiler/src/App.tsx (1)
1-19: LGTM! Clean class component implementation.The component follows React patterns correctly with proper TypeScript typing, lifecycle method usage, and JSX structure. The optional callback pattern in
componentDidMountis implemented safely with optional chaining.packages/testing-library/examples/react-compiler/src/__tests__/expectLogs.jsx (1)
14-17: Globalexpectis available by default in Vitest testsVitest injects the
expectAPI globally, so you don’t need to import it in your test files.Likely an incorrect or invalid review comment.
packages/testing-library/examples/react-compiler/src/__tests__/index.test.tsx (1)
19-19:__MAIN_THREAD__is declared and injected by the testing environmentThe global
__MAIN_THREAD__is defined inpackages/testing-library/testing-environment/src/index.tsand set onglobalThisby bothinjectMainThreadGlobals(sets it totrue) andinjectBackgroundThreadGlobals(sets it tofalse), which therenderhelper uses automatically. No additional setup is required in the example test.packages/testing-library/examples/react-compiler/src/__tests__/hello.test.jsx (3)
9-20: Excellent React compiler optimization test pattern.The
Hellocomponent effectively demonstrates React compiler behavior by logging recomputations during the map operation. This is a great pattern for testing whether the compiler is properly optimizing renders.
22-43: Well-structured test setup and initial assertions.The test properly sets up state management, renders the component, and verifies both the rendered output and the recomputation logs. The snapshot testing provides comprehensive coverage of the component structure.
100-102: Good verification of React compiler optimization.The empty logs array after the state update confirms that the React compiler is working correctly - the items array didn't need to be recomputed when only the
nameprop changed, demonstrating proper memoization.
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
packages/testing-library/examples/react-compiler/src/__tests__/index.test.tsx
Show resolved
Hide resolved
packages/testing-library/examples/react-compiler/src/__tests__/index.test.tsx
Show resolved
Hide resolved
packages/testing-library/examples/react-compiler/src/__tests__/index.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/testing-library/examples/react-compiler/src/__tests__/update-button.test.jsx (1)
63-63: Same undefined variable issue here.The
__FORGET__variable is referenced again but remains undefined.
🧹 Nitpick comments (5)
.changeset/tame-forks-agree.md (2)
11-17:configis undefined in the example snippetThe example attempts to merge
defaultConfigwithconfig, butconfigis never declared. Either import / create it (e.g. viadefineConfig({ … })) or remove the merge step to avoid copy-paste errors.-const defaultConfig = await createVitestConfig({ - runtimePkgName: '@lynx-js/react', - experimental_enableReactCompiler: true, -}); - -export default mergeConfig(defaultConfig, config); +export default await createVitestConfig({ + runtimePkgName: '@lynx-js/react', + experimental_enableReactCompiler: true, +});
7-15: Minor wording & punctuation improvementsConsider tightening the prose for clarity:
-Support testing React Compiler in testing library. Enable React Compiler by setting the `experimental_enableReactCompiler` option of `createVitestConfig` to `true`. +Add React Compiler support to the testing library. +Enable it by passing `experimental_enableReactCompiler: true` to `createVitestConfig`.packages/testing-library/examples/react-compiler/src/__tests__/update-button.test.jsx (1)
13-23: Consider test isolation concerns with module-level state.The module-level variables
currentThemeandstyleComputationscould cause issues in parallel test execution or when tests don't run in isolation.Consider resetting state in a
beforeEachhook or using a more isolated approach:+beforeEach(() => { + currentTheme = 'light'; + styleComputations = 0; +});packages/testing-library/examples/react-compiler/src/__tests__/update-expressions.test.jsx (1)
26-26: Test name doesn't match the actual test behavior.The test is named 'use-state' but it doesn't test React state usage. It tests expression evaluation order in increment/decrement operations.
-test('use-state', async () => { +test('expression-evaluation-order', async () => {packages/testing-library/examples/react-compiler/vitest.config.ts (1)
1-1: Remove unused import.The
mergeConfigimport is not used in this file.-import { defineConfig, mergeConfig } from 'vitest/config'; +import { defineConfig } from 'vitest/config';
📜 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 (20)
.changeset/quick-turtles-make.md(1 hunks).changeset/tame-forks-agree.md(1 hunks)packages/react/testing-library/src/__tests__/rerender.test.jsx(3 hunks)packages/react/testing-library/src/pure.jsx(3 hunks)packages/react/testing-library/src/vitest.config.js(3 hunks)packages/react/testing-library/types/entry.d.ts(1 hunks)packages/testing-library/examples/react-compiler/LICENSE(1 hunks)packages/testing-library/examples/react-compiler/package.json(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/constant-prop.test.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/expectLogs.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/hello.test.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/index.test.tsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/update-button.test.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/update-expressions.test.jsx(1 hunks)packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.jsx(1 hunks)packages/testing-library/examples/react-compiler/tsconfig.json(1 hunks)packages/testing-library/examples/react-compiler/vitest.config.compiler-disabled.ts(1 hunks)packages/testing-library/examples/react-compiler/vitest.config.compiler-enabled.ts(1 hunks)packages/testing-library/examples/react-compiler/vitest.config.ts(1 hunks)vitest.workspace.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- .changeset/quick-turtles-make.md
- packages/testing-library/examples/react-compiler/LICENSE
- packages/testing-library/examples/react-compiler/vitest.config.compiler-enabled.ts
- packages/testing-library/examples/react-compiler/vitest.config.compiler-disabled.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/testing-library/examples/react-compiler/package.json
- packages/testing-library/examples/react-compiler/src/tests/index.test.tsx
- packages/react/testing-library/src/vitest.config.js
- packages/testing-library/examples/react-compiler/src/tests/expectLogs.jsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/react/testing-library/src/__tests__/rerender.test.jsx (1)
packages/react/testing-library/src/pure.jsx (1)
render(39-108)
packages/testing-library/examples/react-compiler/src/__tests__/update-expressions.test.jsx (1)
packages/react/testing-library/src/pure.jsx (1)
render(39-108)
packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.jsx (3)
packages/testing-library/examples/react-compiler/src/__tests__/expectLogs.jsx (2)
log(7-9)expectLogsAndClear(11-14)packages/react/testing-library/src/pure.jsx (1)
render(39-108)packages/react/testing-library/src/fire-event.ts (1)
fireEvent(22-38)
🔇 Additional comments (20)
packages/react/testing-library/types/entry.d.ts (1)
100-100: Type definition correctly reflects the new asFragment method.The addition of
asFragment: () => DocumentFragmentto theRenderResulttype is consistent with the implementation inpure.jsxand provides proper TypeScript support.packages/react/testing-library/src/__tests__/rerender.test.jsx (4)
8-8: Correct use of the new asFragment method.The test correctly destructures
asFragmentfrom the render result to enable DOM snapshot testing.
19-21: Properly updated rerender usage.The test correctly uses
rerenderwithout expecting a return value and usesasFragment()to obtain the container snapshot, which is consistent with the updated API.
24-28: Expected snapshot change from page to DocumentFragment.The snapshot correctly shows
<DocumentFragment>instead of<page>sinceasFragment()returns a DocumentFragment containing the rendered content.
46-47: Correct query function usage.The test properly uses
findByTextfrom the initial render result rather than expecting it from the rerender call, which aligns with the updated rerender behavior.packages/testing-library/examples/react-compiler/src/__tests__/hello.test.jsx (3)
8-19: Well-structured React Compiler test component.The
Hellocomponent effectively demonstrates React Compiler behavior by logging recomputations of theitemsarray. The component structure is appropriate for testing optimization scenarios.
21-56: Comprehensive initial render test.The test properly verifies both the rendered output and the expected recomputation logs, providing good coverage of the React Compiler's initial behavior.
90-92: Correct conditional expectation for React Compiler optimization.The test correctly expects no recomputation logs when
__FORGET__(React Compiler) is enabled, and the original logs when disabled, demonstrating the optimization's effectiveness.packages/react/testing-library/src/pure.jsx (4)
46-46: Good addition of isRerender parameter.The
isRerenderparameter allows the function to distinguish between initial renders and rerenders, enabling proper state preservation.
62-69: Correct rerender optimization.The conditional logic properly preserves the existing
<page/>element during rerenders by skipping main thread switching and root JSX resetting, which improves performance and maintains state consistency.
82-92: Properly implemented rerender method.The
rerendermethod correctly calls the mainrenderfunction withisRerender: trueand doesn't return anything, maintaining API consistency as documented in the comments.
93-105: Robust asFragment implementation with fallback.The
asFragmentmethod provides a solid implementation that usesdocument.createRange()for modern browsers and falls back to<template>element for older browsers, ensuring broad compatibility.packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.jsx (3)
9-25: Well-designed Counter component for React Compiler testing.The
Countercomponent effectively demonstrates useState behavior and includes aTitlecomponent that logs renders to track React Compiler optimizations.
30-70: Appropriate conditional snapshot testing.The conditional snapshots based on
__FORGET__correctly show the different rendering behavior with and without React Compiler, where the compiler eliminates the extra wrapper element.
74-78: Correct interaction testing and optimization verification.The test properly simulates user interaction with
fireEvent.tapand correctly expects different rerender behavior based on React Compiler enablement, demonstrating the optimization's effectiveness.packages/testing-library/examples/react-compiler/src/__tests__/constant-prop.test.jsx (1)
1-130: Excellent test coverage for React Compiler constant propagation.The test suite comprehensively covers different scenarios of constant propagation optimization including literal constants, global constants, lambda closures, and phi nodes with control flow. The use of
'use memo'directive and snapshot testing provides good verification of React Compiler behavior.packages/testing-library/examples/react-compiler/src/__tests__/update-expressions.test.jsx (1)
26-44: Move expression evaluation assertions outside of render cycle.The test should verify the expression evaluation behavior separately from the rendering behavior.
test('expression-evaluation-order', async () => { + // Test expression evaluation separately + testExpressionEvaluation(0); + testExpressionEvaluation(1); + const { asFragment, rerender } = render(<Counter value={0} />); expect(asFragment()).toMatchInlineSnapshot(` <DocumentFragment> <text> 0 </text> </DocumentFragment> `); rerender(<Counter value={1} />); expect(asFragment()).toMatchInlineSnapshot(` <DocumentFragment> <text> 1 </text> </DocumentFragment> `); });Likely an incorrect or invalid review comment.
packages/testing-library/examples/react-compiler/tsconfig.json (1)
1-11: React compiler example tsconfig.json is correctly configuredThe
extendspath resolves properly (basetsconfig.jsonexists at the repository root), and the JSX settings ("react-jsx"withjsxImportSource: "@lynx-js/react") together withnoEmit: trueandinclude: ["src"]are appropriate for this example. No changes required.packages/testing-library/examples/react-compiler/vitest.config.ts (1)
3-10: Referenced Vitest project config files verifiedBoth referenced config files were found under
packages/testing-library/examples/react-compiler:
- vitest.config.compiler-enabled.ts
- vitest.config.compiler-disabled.ts
No further action required.
vitest.workspace.json (1)
7-8: Adjust workspace globs to truly exclude the main configOur verification shows that the glob
packages/.../vitest.config.*.tsstill matchesvitest.config.ts(since*can be empty), and because the exclude pattern is listed first, it never takes effect. To fix this, either swap the patterns so exclusion runs last, or explicitly list the two compiler configs.• File: vitest.workspace.json (around lines 7–8)
• Problem:
"vitest.config.*.ts"matches all three files (includingvitest.config.ts),
and the leading!…vitest.config.tsplaced before it does nothing.• Fix options (choose one):
- Swap include/exclude order:
- "!packages/testing-library/examples/react-compiler/vitest.config.ts", - "packages/testing-library/examples/react-compiler/vitest.config.*.ts", + "packages/testing-library/examples/react-compiler/vitest.config.*.ts", + "!packages/testing-library/examples/react-compiler/vitest.config.ts",
- Or be explicit about the two configs:
- "!packages/testing-library/examples/react-compiler/vitest.config.ts", - "packages/testing-library/examples/react-compiler/vitest.config.*.ts", + "packages/testing-library/examples/react-compiler/vitest.config.compiler-enabled.ts", + "packages/testing-library/examples/react-compiler/vitest.config.compiler-disabled.ts",Either change will ensure the main
vitest.config.tsis excluded and only the two example configs are picked up.Likely an incorrect or invalid review comment.
packages/testing-library/examples/react-compiler/src/__tests__/update-button.test.jsx
Show resolved
Hide resolved
packages/testing-library/examples/react-compiler/src/__tests__/update-expressions.test.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.md (1)
71-71: Missing release-tag / docblock for new experimental flag
experimental_enableReactCompileris surfaced in the public API but carries no@alpha/@betarelease-tag or explanatory JSDoc.
Please annotate the source declaration with an explicit release tag and a short description; otherwise API-Extractor will continue to emit warnings and consumers will have no guidance on stability expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.md(1 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(4 hunks)packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md
Outdated
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #1269 will degrade performances by 7.92%Comparing Summary
Benchmarks breakdown
Footnotes
|
React Example#6113 Bundle Size — 237.45KiB (0%).a52d7b6(current) vs 04f2c39 main#6104(baseline) Bundle metrics
|
| Current #6113 |
Baseline #6104 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
165 |
165 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
46.77% |
46.77% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6113 |
Baseline #6104 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.69KiB |
91.69KiB |
Bundle analysis report Branch upupming:feat/react-compiler Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#6109 Bundle Size — 366.7KiB (0%).a52d7b6(current) vs 04f2c39 main#6100(baseline) Bundle metrics
|
| Current #6109 |
Baseline #6100 |
|
|---|---|---|
| Initial JS |
146.2KiB |
146.2KiB |
| Initial CSS |
32.22KiB |
32.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
221 |
221 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.21% |
3.21% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6109 |
Baseline #6100 |
|
|---|---|---|
| JS |
240.68KiB |
240.68KiB |
| Other |
93.8KiB |
93.8KiB |
| CSS |
32.22KiB |
32.22KiB |
Bundle analysis report Branch upupming:feat/react-compiler Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/quick-turtles-make.md(1 hunks)examples/react-compiler/lynx.config.ts(1 hunks)examples/react-compiler/package.json(1 hunks)packages/react/testing-library/src/vitest.config.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/quick-turtles-make.md
- examples/react-compiler/lynx.config.ts
🧰 Additional context used
🧠 Learnings (2)
📚 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/testing-library/src/vitest.config.js
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
examples/react-compiler/package.json
🧬 Code graph analysis (1)
packages/react/testing-library/src/vitest.config.js (1)
packages/react/testing-library/loaders/jsx-loader.js (2)
sourcePath(10-10)result(17-41)
🔇 Additional comments (8)
examples/react-compiler/package.json (3)
1-9: Package structure and scripts are well-configured.The package is properly set up as a private ESM module with appropriate build and dev scripts using rspeedy. Workspace references are correctly specified.
10-13: No issues found—@types/react versions are already aligned.The @types/react version in examples/react-compiler/package.json (^18.3.25) matches the version declared in @lynx-js/react's devDependencies (^18.3.25). Additionally, @lynx-js/react declares ^18 as a peerDependency, which is compatible with ^18.3.25. The alignment is correct.
22-24: Version pinning inconsistency is not a concern — @swc/react-compiler and babel-plugin-react-compiler are alternative compiler implementations.These packages are mutually exclusive tools for different build systems (SWC vs. Babel), not interdependent components. The babel-plugin-react-compiler is correctly paired with react-compiler-runtime (both pinned to 0.0.0-experimental-0566679-20250709), while @swc/react-compiler independently manages its own versioning via semver. No version parity is needed between them, and no reproducibility issue exists.
Likely an incorrect or invalid review comment.
packages/react/testing-library/src/vitest.config.js (5)
64-66: LGTM!The extension replacement logic for Preact aliases is straightforward and correct.
258-262: LGTM!The conditional plugin inclusion based on the experimental flag is implemented correctly.
271-271: LGTM!The alias array correctly includes the new React and React Compiler runtime aliases.
117-122: Fix the error message template string syntax.The template string is not constructed correctly. You're using a regular string with
${...}placeholder, but the${...}is outside the template literal.Apply this diff:
- if (missingBabelPackages.length > 0) { - throw `With \`experimental_enableReactCompiler\` enabled, you need to install \`${ - missingBabelPackages.join( - '`, `', - ) - }\` in your project root to use React Compiler.`; - } + if (missingBabelPackages.length > 0) { + throw new Error( + `With \`experimental_enableReactCompiler\` enabled, you need to install \`${missingBabelPackages.join('`, `')}\` in your project root to use React Compiler.` + ); + }Likely an incorrect or invalid review comment.
75-92: Verify that hardcodingsrc/index.tsis necessary and works in all build scenarios.The config's
server.fs.allowenables Vitest to serve TypeScript files in development. However, aliasing directly tosrc/index.ts(a package-internal path) is brittle and bypasses the package's official entry point. The comment indicates this was intentional to enable React aliasing, but this approach:
- Depends on package internals rather than the declared export
- May not optimize correctly in production builds (no
optimizeDeps.include)- Could break if the package restructures its source layout
Suggested improvement: Use the package's proper entry point via
require.resolve('react-compiler-runtime')instead of hardcodingsrc/index.ts, unless there's a specific technical reason preventing it. Ifsrc/index.tsis required, document why in a code comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/sour-weeks-wait.md(1 hunks)packages/react/testing-library/src/vitest.config.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/sour-weeks-wait.md
🧰 Additional context used
🧠 Learnings (1)
📚 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/testing-library/src/vitest.config.js
🧬 Code graph analysis (1)
packages/react/testing-library/src/vitest.config.js (1)
packages/react/testing-library/loaders/jsx-loader.js (2)
sourcePath(10-10)result(17-41)
⏰ 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). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (4)
packages/react/testing-library/src/vitest.config.js (4)
75-92: react-compiler-runtime alias: OK, guarded and scoped.The guarded alias to TS entry is fine and low-risk for consumers that don’t have the package installed.
250-254: Plugin ordering looks right.React Compiler runs before ReactLynx only when enabled—sensible default.
64-66: I'll verify this by checking the codebase for the preact version and then searching for preact export format documentation.Preact .js → .mjs alias change is appropriate—this is a known workaround for ESM interop.
Preact ships ESM files with .js extensions (not .mjs), which causes Node to treat them as CommonJS, resulting in errors like "Named export 'options' not found." Renaming to .mjs or setting "type": "module" resolves the issue. Your alias replacement is a practical solution to work around this limitation.
263-263: Review comment is incorrect—no alias collisions exist.The concern about
preactAliasoverridingreactAliasis unfounded. ThegenerateAlias()function creates module-prefixed patterns (e.g.,/^preact.../), sopreactAliascan never produce a/^react$/pattern. Current merge order poses no conflict.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/react/testing-library/src/vitest.config.js (2)
67-74: Add jsx-runtime and jsx-dev-runtime aliases (previously flagged).This was identified in a previous review but remains unaddressed. The current config only aliases bare
react, but transformed JSX code will import fromreact/jsx-runtimeandreact/jsx-dev-runtime. Without these aliases, module resolution will fail. Other vitest configs in the monorepo already follow this pattern.Apply this diff:
const reactAlias = [ { find: /^react$/, replacement: require.resolve(runtimeOSSPkgName, { paths: [runtimeDir], }), }, + { + find: /^react\/jsx-runtime$/, + replacement: require.resolve(path.posix.join(runtimeOSSPkgName, 'jsx-runtime'), { + paths: [runtimeDir], + }), + }, + { + find: /^react\/jsx-dev-runtime$/, + replacement: require.resolve(path.posix.join(runtimeOSSPkgName, 'jsx-dev-runtime'), { + paths: [runtimeDir], + }), + }, ];
118-124: Throw Error object instead of string (previously flagged).This was identified in a previous review but remains unaddressed. Throwing a string instead of an Error object prevents proper stack traces and error handling. The webpack loader correctly throws
new Error(...)for the same scenario.Apply this diff:
if (missingBabelPackages.length > 0) { - throw `With \`experimental_enableReactCompiler\` enabled, you need to install \`${ - missingBabelPackages.join( - '`, `', - ) - }\` in your project root to use React Compiler.`; + throw new Error( + `With \`experimental_enableReactCompiler\` enabled, you need to install \`${ + missingBabelPackages.join('`, `') + }\` in your project root to use React Compiler.` + ); }
🧹 Nitpick comments (3)
packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.tsx (3)
7-8: Consider adding type definitions for the expectLogs module.The
@ts-ignoredirective suppresses type checking for this import. For better type safety and developer experience, consider adding a.d.tsfile forexpectLogs.jsxor converting it to TypeScript.
51-52: Clarify the comment about the wrapper element.The comment "the Counter component can be compressed to a single snapshot" is unclear. Consider revising to better explain why the extra
<wrapper>element appears without React Compiler.For example:
- // Without React Compiler, the Counter component can be compressed to - // a single snapshot, so there will be an extra wrapper element + // Without React Compiler, the rendering behavior differs and + // produces an extra wrapper element around the Title component
82-84: Consider documenting the__FORGET__global.While the type declaration is correct, adding a comment explaining that this global is set by the build configuration or test environment would improve maintainability.
For example:
+// __FORGET__ is set by the build configuration to indicate whether +// the React Compiler (babel-plugin-react-compiler) is enabled declare global { var __FORGET__: boolean; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/react/testing-library/src/vitest.config.js(3 hunks)packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.tsx(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/testing-library/src/vitest.config.js
🧬 Code graph analysis (3)
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
packages/react/testing-library/src/vitest.config.js (1)
require(9-9)
packages/react/testing-library/src/vitest.config.js (1)
packages/react/testing-library/loaders/jsx-loader.js (2)
sourcePath(10-10)result(17-41)
packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.tsx (1)
packages/testing-library/examples/react-compiler/src/__tests__/expectLogs.jsx (1)
expectLogsAndClear(11-14)
⏰ 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). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: test-rust / clippy
🔇 Additional comments (4)
packages/testing-library/examples/react-compiler/src/__tests__/use-state.test.tsx (3)
11-22: LGTM!The Counter component implementation is correct and demonstrates the React Compiler behavior effectively with a simple useState pattern.
24-27: LGTM!The Title component effectively serves as a render tracker for validating React Compiler optimizations.
29-80: Test logic correctly validates React Compiler optimizations.The test properly verifies that:
- With React Compiler (
__FORGET__ = true): Title component doesn't re-render when Counter state changes (empty logs after tap)- Without React Compiler (
__FORGET__ = false): Title component re-renders unnecessarily (logs "rendering: Counter" again)This effectively demonstrates the memoization benefits of React Compiler.
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
1-97: LGTM! The webpack loader implementation is well-structured.The React Compiler loader correctly implements the webpack loader pattern with proper async handling, conditional transformation based on file type and compiler requirements, and comprehensive error handling. Past review feedback has been addressed, including improved validation, TypeScript support for .tsx files, and null filtering in the plugin array.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Yiming Li <yimingli.cs@gmail.com>
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
vitest.config.ts (1)
73-74: LGTM! Pattern correctly handles multiple Vitest configurations.The exclude-then-include pattern appropriately manages the react-compiler example's multiple Vitest project configurations (compiler-enabled and compiler-disabled variants). The exclusion on line 73 prevents the base config from being discovered, while line 74 includes only the specific variant configs.
Consider adding a brief comment to document why this example requires special handling:
'packages/testing-library/examples/*/vitest.config.ts', + // React Compiler example uses separate config files for compiler-enabled/disabled variants '!packages/testing-library/examples/react-compiler/vitest.config.ts', 'packages/testing-library/examples/react-compiler/vitest.config.*.ts',
📜 Review details
Configuration used: Path: .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 (1)
vitest.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:52-72
Timestamp: 2025-08-13T11:36:12.075Z
Learning: The lynx-stack project requires Node.js >=22 as specified in package.json engines, so Node.js compatibility fallbacks for features introduced before v22 are unnecessary.
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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:
vitest.config.ts
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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:
vitest.config.ts
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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:
vitest.config.ts
📚 Learning: 2025-10-29T10:28:27.519Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_static_extract_dynamic_inline_style.js:20-24
Timestamp: 2025-10-29T10:28:27.519Z
Learning: Files inside packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/ are auto-generated test snapshot files and should not be manually updated. Any issues with the generated code should be addressed in the code generator/transform logic, not in the snapshots themselves.
Applied to files:
vitest.config.ts
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/webpack/react-webpack-plugin/package.json (1)
43-56: Re: Babel packages in devDependenciesAs noted in the previous review, the loader dynamically requires these packages at runtime. However, the loader implementation uses
require.resolve(name, { paths: [rootContext] })to explicitly resolve them from the user's project, not from the plugin's dependencies. This means the current placement indevDependenciesserves development/testing of the plugin itself, while end-users are expected to install these packages in their own projects (enforced by the runtime error in lines 46-52 of the loader).That said, declaring them as
peerDependencies(withpeerDependenciesMetafor optional features) would be more idiomatic and provide better DX through package manager warnings. However, the experimental nature ofbabel-plugin-react-compilercomplicates this.
🧹 Nitpick comments (2)
packages/react/testing-library/src/vitest.config.js (1)
182-182: AddallowDeclareFields: truefor full TypeScript compatibility.The previous review comment suggested including
allowDeclareFields: truealongsideisTSX: truefor the TypeScript syntax plugin. While uncommon in typical React components, some TypeScript code usesdeclarefields in classes, and omitting this option may cause parsing failures in such cases.Apply this diff:
- isTSX ? [babelPluginSyntaxTypescriptPath, { isTSX: true }] : null, + isTSX ? [babelPluginSyntaxTypescriptPath, { isTSX: true, allowDeclareFields: true }] : null,packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (1)
94-106: Handle input source maps for proper chainingThe loader generates output source maps but doesn't consume input source maps from previous loaders in the chain. This can break source map continuity when multiple loaders are used.
Consider passing input source maps to Babel:
+ // Get input source map from previous loader if available + const inputSourceMap = this.sourceMap && this.callback.length >= 3 + ? undefined // webpack will provide it; check loader context API + : undefined; + const result = babel.transformSync(content, { plugins: [ [babelPluginReactCompilerPath, { target: '17' }], babelPluginSyntaxJsxPath, isTSX ? [babelPluginSyntaxTypescriptPath, { isTSX: true }] : null, ].filter(Boolean) as PluginItem[], filename: this.resourcePath, ast: false, sourceMaps: this.sourceMap, + inputSourceMap, });Note: The exact method to retrieve input source maps depends on how webpack/rspack provides them to loaders in your setup.
📜 Review details
Configuration used: Path: .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 (5)
packages/react/testing-library/src/vitest.config.js(3 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(5 hunks)packages/rspeedy/plugin-react/src/reactCompiler.ts(1 hunks)packages/webpack/react-webpack-plugin/package.json(1 hunks)packages/webpack/react-webpack-plugin/src/loaders/react-compiler.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 (22)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:52-72
Timestamp: 2025-08-13T11:36:12.075Z
Learning: The lynx-stack project requires Node.js >=22 as specified in package.json engines, so Node.js compatibility fallbacks for features introduced before v22 are unnecessary.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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
Repo: lynx-family/lynx-stack PR: 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: PupilTong
Repo: lynx-family/lynx-stack PR: 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.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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.
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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/testing-library/src/vitest.config.js
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-11T06:00:04.376Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-07-18T04:27:18.291Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-23T08:53:56.927Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/src/loader.ts:244-251
Timestamp: 2025-09-23T08:53:56.927Z
Learning: In webpack CSS extraction plugins, when storing per-module dependencies in a compiler-scoped map like cssModuleId2Deps, the map should not be reset at compilation start because in incremental compilation (watch mode/HMR), only changed files pass through the loader. Unchanged modules need their dependency information to persist between compilations so the plugin can access all modules' dependencies when generating CSS output.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-10-29T10:28:27.519Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_static_extract_dynamic_inline_style.js:20-24
Timestamp: 2025-10-29T10:28:27.519Z
Learning: Files inside packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/ are auto-generated test snapshot files and should not be manually updated. Any issues with the generated code should be addressed in the code generator/transform logic, not in the snapshots themselves.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/rspeedy/plugin-react/src/reactCompiler.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/react/testing-library/src/vitest.config.jspackages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
packages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
packages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/webpack/react-webpack-plugin/package.json
📚 Learning: 2025-08-13T11:36:12.075Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:52-72
Timestamp: 2025-08-13T11:36:12.075Z
Learning: The lynx-stack project requires Node.js >=22 as specified in package.json engines, so Node.js compatibility fallbacks for features introduced before v22 are unnecessary.
Applied to files:
packages/webpack/react-webpack-plugin/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). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (9)
packages/react/testing-library/src/vitest.config.js (6)
64-66: LGTM!The preact alias postprocessing correctly converts
.jsreferences to.mjsfor ESM compatibility.
67-86: LGTM!The react aliases correctly cover
react,react/jsx-runtime, andreact/jsx-dev-runtime, aligning with rspeedy's behavior as confirmed in previous discussions.
88-132: LGTM!The dependency resolution correctly checks for all required packages and throws a proper
Errorobject with a clear message, addressing previous review comments.
134-209: Well-structured plugin with proper caching and error handling.The plugin correctly:
- Caches dependencies in the
confighook for reuse across transforms- Optimizes by checking
isReactCompilerRequiredbefore transforming- Handles errors comprehensively with clear messages
- Returns proper types for Vite plugin contracts
291-295: LGTM!The conditional plugin inclusion correctly uses optional chaining and integrates the React Compiler transform before the ReactLynx transform.
304-304: LGTM!The alias array correctly includes all necessary aliases, with the react-compiler-runtime alias being added dynamically in the plugin's config hook.
packages/rspeedy/plugin-react/src/reactCompiler.ts (1)
8-19: LGTM!The function correctly wires the React Compiler loader into the bundler chain. The regex
/\.[jt]sx$/appropriately targets only.jsxand.tsxfiles, which is the expected convention for files containing JSX.packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts (2)
21-62: Well-structured dependency resolutionThe function correctly resolves compiler dependencies from the user's project root using
require.resolvewith explicit paths, and provides clear error messaging when packages are missing.
96-99: Verify target '17' is still the intended configurationThe comment explains that target
'17'is deliberately chosen to usereact-compiler-runtimeinstead ofreact/compiler-runtimefor Preact compatibility. However, a previous review suggested updating to'19'to match the project's React version (19.1.0).Please confirm whether:
- The runtime package difference (
react-compiler-runtimevsreact/compiler-runtime) is still the correct approach for your use case- Target
'17'continues to provide the needed compatibility despite using React 19If this configuration is correct, consider adding a more explicit comment explaining that the target value controls the runtime package import path, not React version compatibility.
There was a problem hiding this comment.
I'm not sure if implementing a loader by ourself is a good idea.
Here are some options:
experimental_enableReactCompilerwith our own loader (the current implementation)- Some of the babel configs (
babel-plugin-react-compiler, JSX syntax) are hard-coded in our loader - Others should be placed in the
.babelrcin the project
- Some of the babel configs (
experimental_enableReactCompilerwithreact-compiler-webpack@rsbuild/pluigin-babelinlynx.config.js(the recommended way of Rsbuild: https://rsbuild.rs/guide/framework/react#react-compiler)- All the babel configs can be placed in the option of
@rsbuild/pluigin-babel - AST can be re-used with the existing babel plugins (if there are any)
- All the babel configs can be placed in the option of
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/src/loaders/react-compiler.ts
Outdated
Show resolved
Hide resolved
upupming
force-pushed
the
feat/react-compiler
branch
from
7c2d240 to
eb47053
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/react/testing-library/src/vitest.config.js (1)
88-202: Missing theisReactCompilerRequiredcheck for selective transformation.According to the PR objectives, the implementation should "use swc binding support (binding_react_compiler_node/src/support.rs) to determine whether the babel-plugin-react-compiler transform is necessary." The current code transforms ALL
.jsxand.tsxfiles without checking if React Compiler transformation is actually needed, which is inefficient and inconsistent with the stated design.Earlier review iterations included this check using
@swc/react-compiler'sisReactCompilerRequiredfunction. This selective transformation should be restored.Add the selective transformation check:
function transformReactCompilerPlugin() { let rootContext, compilerDeps, babel; function resolveCompilerDeps(rootContext) { const missingBabelPackages = []; const [ + swcReactCompilerPath, babelPath, babelPluginReactCompilerPath, babelPluginSyntaxJsxPath, babelPluginSyntaxTypescriptPath, ] = [ + '@swc/react-compiler', '@babel/core', 'babel-plugin-react-compiler', '@babel/plugin-syntax-jsx', '@babel/plugin-syntax-typescript', ].map((name) => { try { return require.resolve(name, { paths: [rootContext], }); } catch { missingBabelPackages.push(name); } return ''; }); if (missingBabelPackages.length > 0) { throw new Error( `With \`experimental_enableReactCompiler\` enabled, you need to install \`${ missingBabelPackages.join( '`, `', ) }\` in your project root to use React Compiler.`, ); } return { + swcReactCompilerPath, babelPath, babelPluginReactCompilerPath, babelPluginSyntaxJsxPath, babelPluginSyntaxTypescriptPath, }; } return { name: 'transformReactCompilerPlugin', enforce: 'pre', config(config) { rootContext = config.root; const reactCompilerRuntimeAlias = []; try { reactCompilerRuntimeAlias.push( { find: /^react-compiler-runtime$/, replacement: path.join( path.dirname(require.resolve('react-compiler-runtime/package.json', { paths: [rootContext], })), // Use ts to ensure `react` can be aliased to `@lynx-js/react` 'src', 'index.ts', ), }, ); } catch (e) { // console.log('react-compiler-runtime not found, skip alias'); } config.test.alias.push(...reactCompilerRuntimeAlias); compilerDeps = resolveCompilerDeps(rootContext); - const { babelPath } = compilerDeps; + const { swcReactCompilerPath, babelPath } = compilerDeps; babel = require(babelPath); + const { isReactCompilerRequired } = require(swcReactCompilerPath); + this.isReactCompilerRequired = isReactCompilerRequired; }, async transform(sourceText, sourcePath) { if (/\.(?:jsx|tsx)$/.test(sourcePath)) { + const needReactCompiler = await this.isReactCompilerRequired(Buffer.from(sourceText)); + if (!needReactCompiler) { + return null; + } + const { babelPluginReactCompilerPath, babelPluginSyntaxJsxPath, babelPluginSyntaxTypescriptPath } = compilerDeps; const isTSX = sourcePath.endsWith('.tsx'); try { const result = babel.transformSync(sourceText, { plugins: [ // We use '17' to make `babel-plugin-react-compiler` compiles our code // to use `react-compiler-runtime` instead of `react/compiler-runtime` // for the `useMemoCache` hook [babelPluginReactCompilerPath, { target: '17' }], babelPluginSyntaxJsxPath, isTSX ? [babelPluginSyntaxTypescriptPath, { isTSX: true }] : null, ].filter(Boolean), filename: sourcePath, ast: false, sourceMaps: true, }); if (result?.code != null && result?.map != null) { return { code: result.code, map: result.map, }; } else { this.error( `babel-plugin-react-compiler transform failed for ${sourcePath}: ${ result ? 'missing code or map' : 'no result' }`, ); } } catch (e) { this.error(e); } } return null; }, }; }
📜 Review details
Configuration used: Path: .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 (6)
.changeset/sour-weeks-wait.md(1 hunks)examples/react-compiler/lynx.config.ts(1 hunks)examples/react-compiler/package.json(1 hunks)packages/react/testing-library/src/vitest.config.js(3 hunks)packages/rspeedy/plugin-react/src/pluginReactLynx.ts(1 hunks)packages/testing-library/examples/react-compiler/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- packages/testing-library/examples/react-compiler/package.json
- examples/react-compiler/package.json
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/sour-weeks-wait.md
🧠 Learnings (24)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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:
examples/react-compiler/lynx.config.tspackages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
examples/react-compiler/lynx.config.tspackages/react/testing-library/src/vitest.config.js.changeset/sour-weeks-wait.md
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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:
examples/react-compiler/lynx.config.tspackages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
examples/react-compiler/lynx.config.tspackages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
examples/react-compiler/lynx.config.tspackages/react/testing-library/src/vitest.config.js.changeset/sour-weeks-wait.md
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
examples/react-compiler/lynx.config.ts
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
examples/react-compiler/lynx.config.ts.changeset/sour-weeks-wait.md
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
examples/react-compiler/lynx.config.ts
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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.
Applied to files:
examples/react-compiler/lynx.config.tspackages/react/testing-library/src/vitest.config.js.changeset/sour-weeks-wait.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
examples/react-compiler/lynx.config.ts
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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/testing-library/src/vitest.config.js.changeset/sour-weeks-wait.md
📚 Learning: 2025-08-11T06:00:04.376Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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/testing-library/src/vitest.config.js
📚 Learning: 2025-09-23T08:53:56.927Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/src/loader.ts:244-251
Timestamp: 2025-09-23T08:53:56.927Z
Learning: In webpack CSS extraction plugins, when storing per-module dependencies in a compiler-scoped map like cssModuleId2Deps, the map should not be reset at compilation start because in incremental compilation (watch mode/HMR), only changed files pass through the loader. Unchanged modules need their dependency information to persist between compilations so the plugin can access all modules' dependencies when generating CSS output.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-10-29T10:28:27.519Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_static_extract_dynamic_inline_style.js:20-24
Timestamp: 2025-10-29T10:28:27.519Z
Learning: Files inside packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/ are auto-generated test snapshot files and should not be manually updated. Any issues with the generated code should be addressed in the code generator/transform logic, not in the snapshots themselves.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/react/testing-library/src/vitest.config.js
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/sour-weeks-wait.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/sour-weeks-wait.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/sour-weeks-wait.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/sour-weeks-wait.md
🧬 Code graph analysis (2)
examples/react-compiler/lynx.config.ts (3)
examples/react/lynx.config.js (1)
enableBundleAnalysis(5-5)packages/rspeedy/plugin-react/src/pluginReactLynx.ts (1)
pluginReactLynx(282-400)packages/rspeedy/plugin-qrcode/src/index.ts (1)
pluginQRCode(91-167)
packages/react/testing-library/src/vitest.config.js (1)
packages/react/testing-library/loaders/jsx-loader.js (2)
sourcePath(10-10)result(17-41)
⏰ 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). (5)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: test-rust / clippy
- GitHub Check: zizmor
🔇 Additional comments (4)
packages/react/testing-library/src/vitest.config.js (4)
64-66: LGTM! Preact MJS module mapping is correct.The replacement of
.jswith.mjsfor Preact aliases correctly aligns with Preact's ESM distribution format.
67-86: LGTM! React alias mappings are complete.The three alias entries correctly map
react,react/jsx-runtime, andreact/jsx-dev-runtimeto the Lynx React runtime, aligning with rspeedy's behavior per the earlier discussion.
285-289: LGTM! Conditional plugin registration is correct.The React Compiler plugin is correctly gated by the
experimental_enableReactCompilerflag and positioned before the Lynx transform plugin, ensuring the proper transformation order.
298-298: LGTM! Alias array correctly includes React aliases.The addition of
reactAliasto the test configuration's alias array properly enables React import resolution to the Lynx runtime.
| opts.plugins?.unshift([ | ||
| 'babel-plugin-react-compiler', | ||
| // See https://react.dev/reference/react-compiler/configuration for config | ||
| { | ||
| // ReactLynx only supports target to version 17 | ||
| target: '17', | ||
| }, | ||
| ]); | ||
| }, |
There was a problem hiding this comment.
Ensure the React Compiler plugin is always registered.
Line 16 currently calls opts.plugins?.unshift(...), but Babel passes opts.plugins as undefined when no plugins were configured, so the React Compiler transform never gets added. Initialize the array before unshifting to guarantee the plugin runs.
babelLoaderOptions(opts) {
- opts.plugins?.unshift([
+ opts.plugins ??= [];
+ opts.plugins.unshift([🤖 Prompt for AI Agents
In examples/react-compiler/lynx.config.ts around lines 15 to 23, opts.plugins is
unshifted when it may be undefined so the React Compiler plugin can be skipped;
ensure opts.plugins is initialized to an array when falsy (e.g., set
opts.plugins = opts.plugins || []) before adding the plugin, then add the
'babel-plugin-react-compiler' entry with the target: '17' configuration so the
transform is always registered.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rspeedy/plugin-react/test/config.test.ts (1)
2805-2825: Consider relocating test to the main "Config" describe block.This test verifies a general default behavior (dedupe configuration) that applies to all Rspeedy configurations, not just MPA scenarios. For better organization and clarity, consider moving it to the main "Config" describe block (starting at line 19) alongside other general default configuration tests like "enableRemoveCSSScope defaults to true" (line 370).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rspeedy/plugin-react/test/config.test.ts(1 hunks)packages/testing-library/examples/react-compiler/lynx.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/testing-library/examples/react-compiler/lynx.config.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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/rspeedy/plugin-react/test/config.test.ts
📚 Learning: 2025-10-29T10:28:27.519Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_static_extract_dynamic_inline_style.js:20-24
Timestamp: 2025-10-29T10:28:27.519Z
Learning: Files inside packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/ are auto-generated test snapshot files and should not be manually updated. Any issues with the generated code should be addressed in the code generator/transform logic, not in the snapshots themselves.
Applied to files:
packages/rspeedy/plugin-react/test/config.test.ts
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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/rspeedy/plugin-react/test/config.test.ts
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/plugin-react/test/config.test.ts
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 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.
Applied to files:
packages/rspeedy/plugin-react/test/config.test.ts
🧬 Code graph analysis (1)
packages/rspeedy/plugin-react/test/config.test.ts (1)
packages/rspeedy/plugin-react/src/pluginReactLynx.ts (1)
pluginReactLynx(282-400)
4 participants
Summary
Support react compiler for ReactLynx.
babel-plugin-react-compileris used to compile tsx/jsxbabel-plugin-react-compilertransform is necessaryreact-compiler-runtimeis compatiable with preact, so we use it directly in the exampleChecklist
Summary by CodeRabbit
New Features
Examples
Testing
Documentation