Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional per-element nodeIndex fields and UI source-map collection across the React transform, N‑API bindings, bundler loaders/plugins, template encode hooks, tests, and an example; threads new flags/options to enable node-index and ui-source-map generation and emits a compact node-index-map.json when enabled. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🦋 Changeset detectedLatest commit: 252f450 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c518bfc906
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
...s/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.js
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
Outdated
Show resolved
Hide resolved
upupming
force-pushed
the
codex/node-index-asset
branch
from
b9a173b to
59f2239
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3c22dcafa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.js (1)
8-16:⚠️ Potential issue | 🟠 MajorPass
enableNodeIndexthroughpluginOptionstoo.
createConfig()only forwards the first argument to the loader rules.ReactWebpackPluginreadsenableNodeIndexfrom the second argument, so this fixture currently collects node-index records but never emitsnode-index-map.json, which will make the new assertions intest.config.cjsfail.Suggested fix
const config = createConfig({ enableNodeIndex: true, }, { + enableNodeIndex: true, mainThreadChunks: [ 'main__main-thread.js', './lazy.jsx-react__main-thread.js', ], experimental_isLazyBundle: true, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.js` around lines 8 - 16, The fixture calls createConfig({ enableNodeIndex: true }, { ... }) but createConfig only forwards the first arg into loader rules while ReactWebpackPlugin reads enableNodeIndex from pluginOptions (the second arg); update the call so enableNodeIndex is included in pluginOptions as well (i.e., pass enableNodeIndex: true inside the second argument used for ReactWebpackPlugin/pluginOptions) so node-index-map.json is emitted; check symbols createConfig and ReactWebpackPlugin/pluginOptions to place the flag correctly.
🧹 Nitpick comments (3)
.github/react-transform.instructions.md (1)
5-9: Clear and technically sound guidelines.The guidelines accurately cover critical aspects of the node-index implementation:
- Proper separation between core Rust and napi types
- Guards for synthetic spans to prevent panics
- Correct column numbering convention
- Span preservation through transformations
- API consistency requirements
📋 Optional: Consider bullet-point formatting for easier scanning
The current dense paragraph format is functional but could be more scannable. Consider:
-When a crate exposes both core Rust structs and `napi` wrapper structs with the same semantic shape, keep internal transform pipelines and shared `Rc<RefCell<...>>` state on the core types and convert to the `napi` types only at the JS boundary. Do not mix `swc_plugin_*::napi::*` record types into internal plugin wiring such as `.with_*_records(...)`, or wasm builds can fail with mismatched type errors. -When recording source locations from SWC spans, guard `SourceMap::lookup_char_pos` for synthetic spans such as `DUMMY_SP` (`span.lo == 0`). Compat and other transforms may synthesize JSX nodes with default spans, and wasm builds can surface panics from source map lookups on those spans as `RuntimeError: unreachable`. -Expose recorded columns as 1-based values so `nodeIndexRecords` can be fed directly into editor locations such as VS Code without an extra offset conversion. -When compat wraps a component with a synthetic `<view>`, preserve the original component spans on the generated wrapper instead of using `DUMMY_SP` or `Default::default()`. Snapshot node-index extraction reads `opening.span`, so preserved spans keep `nodeIndexRecords` file, line, and column data intact. -If `swc_plugin_snapshot::JSXTransformer::new` gains a new constructor parameter, update every external callsite under `packages/react/transform/**` at the same time, including wrapper crates such as `swc-plugin-reactlynx`, not just the main `packages/react/transform/src/lib.rs` entrypoint. +## Core Type Management + +- **Separate napi from core types**: Keep internal transform pipelines and shared `Rc<RefCell<...>>` state on core Rust types. Convert to `napi` types only at the JS boundary. +- **Avoid mixing napi record types** into internal plugin wiring like `.with_*_records(...)` — wasm builds can fail with mismatched type errors. + +## Source Location Handling + +- **Guard synthetic spans**: When recording source locations, check `span.lo == 0` before calling `SourceMap::lookup_char_pos` to avoid `RuntimeError: unreachable` panics from `DUMMY_SP`. +- **Use 1-based columns**: Expose columns as 1-based so `nodeIndexRecords` work directly with editors like VS Code. +- **Preserve original spans**: When compat wraps components with synthetic `<view>`, preserve the original spans instead of using `DUMMY_SP`. This keeps `nodeIndexRecords` location data intact. + +## API Consistency + +- **Sync constructor changes**: If `swc_plugin_snapshot::JSXTransformer::new` signature changes, update all callsites under `packages/react/transform/**` simultaneously, including wrapper crates like `swc-plugin-reactlynx`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/react-transform.instructions.md around lines 5 - 9, Convert the dense guideline paragraph into a concise, scannable bullet-style list while preserving the same technical points: keep internal pipelines on core types and avoid mixing swc_plugin_*::napi::* record types (reference .with_*_records and Rc<RefCell<...>>), guard SourceMap::lookup_char_pos for synthetic spans like DUMMY_SP (span.lo == 0), expose recorded columns as 1-based so nodeIndexRecords map directly to editor locations, preserve original component spans on generated wrappers so opening.span retains file/line/column, and ensure any new parameter added to swc_plugin_snapshot::JSXTransformer::new is updated across all external callsites in packages/react/transform/** (including wrapper crates like swc-plugin-reactlynx); reformat text into short bullets for each of these symbols/requirements.packages/react/transform/crates/swc_plugin_compat/lib.rs (1)
929-935: Usetake()here instead of cloning the whole JSX tree twice.Line 929 and Line 930 both deep-clone the already-transformed component subtree. In
compiler_onlymode this sits on the hot path for every wrapped component, so large nested JSX trees pay for two full copies unnecessarily. SinceTakeis already in scope, movenout, read its spans, and wrap that moved child directly. A smallwrap_with_view_spans(...)helper next towrap_with_view(...)is enough to keep the current span-preservation behavior.♻️ Suggested shape
- let component_jsx = n.clone(); - let wrapped_child = component_jsx.clone(); - *n = self.wrap_with_view( - &component_jsx, + let wrapped_child = n.take(); + let opening_span = wrapped_child.opening.span; + let closing_span = wrapped_child + .closing + .as_ref() + .map(|closing| closing.span) + .unwrap_or(opening_span); + *n = self.wrap_with_view_spans( + wrapped_child.span, + opening_span, + closing_span, primitive_attrs, vec![JSXElementChild::JSXElement(Box::new(wrapped_child))], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/transform/crates/swc_plugin_compat/lib.rs` around lines 929 - 935, Replace the double deep-clone of the JSX node (the current component_jsx and wrapped_child clones of n) with a single move using Take: move n out (use Take::take or n.take()) to obtain the node, capture its spans, and pass the moved node directly into wrap_with_view; add a small helper wrap_with_view_spans(...) alongside wrap_with_view that accepts the previously-captured spans (to preserve span info) and constructs the same JSXElementChild without cloning the subtree twice. Ensure references to n, component_jsx and wrapped_child are removed and wrap_with_view_spans is used in place of the two-clone pattern.examples/react-node-index/lynx.config.ts (1)
165-170: UnnecessaryPromise.resolvewrapper.The
mockUploadNodeIndexMapfunction is synchronous, so wrapping it inPromise.resolveandawaitadds unnecessary overhead. Consider simplifying:- const nodeIndexMapUrl = await Promise.resolve( - mockUploadNodeIndexMap( - args.filenameTemplate, - args.intermediate, - ), - ); + const nodeIndexMapUrl = mockUploadNodeIndexMap( + args.filenameTemplate, + args.intermediate, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-node-index/lynx.config.ts` around lines 165 - 170, The current code wraps the synchronous mockUploadNodeIndexMap call with Promise.resolve and await, which is unnecessary; replace the line so nodeIndexMapUrl is assigned directly from mockUploadNodeIndexMap(args.filenameTemplate, args.intermediate) (no Promise.resolve or await) to simplify and avoid extra overhead while keeping the same value semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/react-node-index/package.json`:
- Around line 1-22: Add a packageManager field to this package.json for
`@lynx-js/example-react-node-index` so it matches the pnpm version used at the
repository root: open the repository root package.json to copy the exact
"pnpm@<version>" string and add a top-level "packageManager" property with that
value in this package.json manifest to keep toolchain resolution consistent.
In `@packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts`:
- Around line 115-120: The emitted mapping tuples currently push
record.lineNumber and record.columnNumber as-is, but the spec requires 0-based
positions; update the code that builds mappings (the array pushed via
mappings.push([...]) so that it uses (record.lineNumber - 1) and
(record.columnNumber - 1) (or Math.max(0, ... - 1) to guard against already-zero
values) when creating the tuple for each record.nodeIndex and sourceIndex,
ensuring line/column are normalized to 0-based before serialization.
---
Outside diff comments:
In
`@packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.js`:
- Around line 8-16: The fixture calls createConfig({ enableNodeIndex: true }, {
... }) but createConfig only forwards the first arg into loader rules while
ReactWebpackPlugin reads enableNodeIndex from pluginOptions (the second arg);
update the call so enableNodeIndex is included in pluginOptions as well (i.e.,
pass enableNodeIndex: true inside the second argument used for
ReactWebpackPlugin/pluginOptions) so node-index-map.json is emitted; check
symbols createConfig and ReactWebpackPlugin/pluginOptions to place the flag
correctly.
---
Nitpick comments:
In @.github/react-transform.instructions.md:
- Around line 5-9: Convert the dense guideline paragraph into a concise,
scannable bullet-style list while preserving the same technical points: keep
internal pipelines on core types and avoid mixing swc_plugin_*::napi::* record
types (reference .with_*_records and Rc<RefCell<...>>), guard
SourceMap::lookup_char_pos for synthetic spans like DUMMY_SP (span.lo == 0),
expose recorded columns as 1-based so nodeIndexRecords map directly to editor
locations, preserve original component spans on generated wrappers so
opening.span retains file/line/column, and ensure any new parameter added to
swc_plugin_snapshot::JSXTransformer::new is updated across all external
callsites in packages/react/transform/** (including wrapper crates like
swc-plugin-reactlynx); reformat text into short bullets for each of these
symbols/requirements.
In `@examples/react-node-index/lynx.config.ts`:
- Around line 165-170: The current code wraps the synchronous
mockUploadNodeIndexMap call with Promise.resolve and await, which is
unnecessary; replace the line so nodeIndexMapUrl is assigned directly from
mockUploadNodeIndexMap(args.filenameTemplate, args.intermediate) (no
Promise.resolve or await) to simplify and avoid extra overhead while keeping the
same value semantics.
In `@packages/react/transform/crates/swc_plugin_compat/lib.rs`:
- Around line 929-935: Replace the double deep-clone of the JSX node (the
current component_jsx and wrapped_child clones of n) with a single move using
Take: move n out (use Take::take or n.take()) to obtain the node, capture its
spans, and pass the moved node directly into wrap_with_view; add a small helper
wrap_with_view_spans(...) alongside wrap_with_view that accepts the
previously-captured spans (to preserve span info) and constructs the same
JSXElementChild without cloning the subtree twice. Ensure references to n,
component_jsx and wrapped_child are removed and wrap_with_view_spans is used in
place of the two-clone pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eeded667-ef19-49f0-a759-952f5c55c54e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
.changeset/soft-pumas-push.md.changeset/ten-lions-accept.md.github/react-transform.instructions.md.github/rspeedy-core.instructions.md.github/webpack-node-index.instructions.mdexamples/react-node-index/lynx.config.tsexamples/react-node-index/package.jsonexamples/react-node-index/src/App.cssexamples/react-node-index/src/App.tsxexamples/react-node-index/src/LazyPanel.tsxexamples/react-node-index/src/index.tsxexamples/react-node-index/src/rspeedy-env.d.tsexamples/react-node-index/tsconfig.jsonpackages/react/transform/__test__/fixture.spec.jspackages/react/transform/crates/swc_plugin_compat/lib.rspackages/react/transform/crates/swc_plugin_list/lib.rspackages/react/transform/crates/swc_plugin_snapshot/lib.rspackages/react/transform/crates/swc_plugin_snapshot/napi.rspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_full_static.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/full_static_children_new_line.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/full_static_children_self_close.jspackages/react/transform/crates/swc_plugins_shared/utils.rspackages/react/transform/index.d.tspackages/react/transform/src/lib.rspackages/react/transform/swc-plugin-reactlynx/index.d.tspackages/react/transform/swc-plugin-reactlynx/src/lib.rspackages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.mdpackages/rspeedy/plugin-react/src/entry.tspackages/rspeedy/plugin-react/src/loaders.tspackages/rspeedy/plugin-react/src/pluginReactLynx.tspackages/rspeedy/plugin-react/test/config.test.tspackages/rspeedy/plugin-react/test/validate.test.tspackages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.mdpackages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.tspackages/webpack/react-webpack-plugin/src/loaders/main-thread.tspackages/webpack/react-webpack-plugin/src/loaders/options.tspackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/index.jsxpackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.jspackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/test.config.cjspackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
Outdated
Show resolved
Hide resolved
Merging this PR will degrade performance by 15.43%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | 002-hello-reactLynx-destroyBackground |
929.8 µs | 682.2 µs | +36.29% |
| ❌ | transform 1000 view elements |
40.4 ms | 47.8 ms | -15.43% |
| ⚡ | basic-performance-scroll-view-100 |
8.7 ms | 8.1 ms | +6.54% |
Comparing codex/node-index-asset (cf55e95) with main (b1ad1b9)
Footnotes
-
21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React MTF Example#383 Bundle Size — 206.12KiB (0%).cf55e95(current) vs b1ad1b9 main#368(baseline) Bundle metrics
|
| Current #383 |
Baseline #368 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
173 |
173 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
45.79% |
45.79% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #383 |
Baseline #368 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
94.89KiB |
94.89KiB |
Bundle analysis report Branch codex/node-index-asset Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7250 Bundle Size — 236.83KiB (0%).cf55e95(current) vs b1ad1b9 main#7235(baseline) Bundle metrics
|
| Current #7250 |
Baseline #7235 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
179 |
179 |
| Duplicate Modules |
70 |
70 |
| Duplicate Code |
46.13% |
46.13% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7250 |
Baseline #7235 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.07KiB |
91.07KiB |
Bundle analysis report Branch codex/node-index-asset Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/test.config.cjs`:
- Around line 72-77: The check for async cleanup is a false positive because
asyncEntries.find uses entry.endsWith('node-index-map.json') and matches
filenames like captured-node-index-map.json; update the detection to compare the
path basename instead of using endsWith (e.g., use path.basename(entry) ===
'node-index-map.json') so only the exact file 'node-index-map.json' triggers the
error. Modify the asyncNodeIndexFile detection (the variable and its find
predicate) to use path.basename from Node's path module to perform an exact
filename match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 028dd4d1-29e9-4edf-b261-887a9e70352d
📒 Files selected for processing (8)
.github/webpack-node-index.instructions.mdpackages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.tspackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.jspackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/test.config.cjspackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
✅ Files skipped from review due to trivial changes (1)
- .github/webpack-node-index.instructions.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.js
- packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
- packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
- packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md
...es/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/test.config.cjs
Outdated
Show resolved
Hide resolved
Web Explorer#8824 Bundle Size — 748.66KiB (0%).cf55e95(current) vs b1ad1b9 main#8809(baseline) Bundle metrics
|
| Current #8824 |
Baseline #8809 |
|
|---|---|---|
| Initial JS |
44.27KiB |
44.27KiB |
| Initial CSS |
2.16KiB |
2.16KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
10 |
10 |
 Modules |
150(+0.67%) |
149 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
35.01%(-0.03%) |
35.02% |
| Packages |
3 |
3 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8824 |
Baseline #8809 |
|
|---|---|---|
| Other |
401.63KiB |
401.63KiB |
| JS |
344.87KiB |
344.87KiB |
| CSS |
2.16KiB |
2.16KiB |
Bundle analysis report Branch codex/node-index-asset Project dashboard
Generated by RelativeCI Documentation Report issue
upupming
force-pushed
the
codex/node-index-asset
branch
from
68eec79 to
85b2894
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/webpack/react-webpack-plugin/src/loaders/options.ts (1)
166-176:⚠️ Potential issue | 🔴 CriticalUse the correct snapshot config key
enableUiSourceMapinstead ofenableNodeIndex.The snapshot config object is typed as
JsxTransformerConfig, which definesenableUiSourceMap?: boolean, notenableNodeIndex. The current code violates TypeScript strict mode and prevents the flag from reaching the transform layer. Apply the fix:- enableNodeIndex: enableNodeIndex ?? false, + enableUiSourceMap: enableNodeIndex ?? false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/react-webpack-plugin/src/loaders/options.ts` around lines 166 - 176, The snapshot config uses the wrong key: replace the incorrect snapshot property enableNodeIndex with enableUiSourceMap so the flag matches the JsxTransformerConfig type and flows to the transform layer; locate the snapshot object construction (the object containing preserveJsx, target, runtimePkg) in the options loader (see the snapshot variable/block in options.ts) and change enableNodeIndex ?? false to enableUiSourceMap ?? false, keeping the rest of the logic intact so TypeScript strict mode and downstream consumers (JsxTransformerConfig) see the correct flag.
🧹 Nitpick comments (1)
.github/rspeedy-core.instructions.md (1)
5-5: Consider adding more context about the cleanup logic being referenced.While the instruction is clear about what to preserve (git-tracked fixture files under
node_modules), it could benefit from mentioning:
- Which cleanup script/tool this applies to
- Whether this is for manual developer reference or automated tooling
- An example of files that should be preserved vs. removed
Also, the LanguageTool hint flagging "rspeedy" is a false positive—it's a valid product name in this codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/rspeedy-core.instructions.md at line 5, Clarify the cleanup guidance by naming the cleanup script/tool (e.g., the repo's clean/build script or CI job that runs cache cleanup), state whether the note targets developers or automated tooling, and add a brief example showing a tracked fixture file to preserve (packages/rspeedy/core/test/<fixture>/node_modules/some-tracked-file.js) versus generated artifacts to delete (packages/rspeedy/core/test/<fixture>/node_modules/.cache/* or node_modules/<pkg>-build-output), and also note that the LanguageTool flag for "rspeedy" is a false positive and should be ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react/transform/crates/swc_plugin_snapshot/lib.rs`:
- Around line 322-359: The list branch currently calls snapshotCreateList(...)
directly and therefore bypasses the UI source-map payload; modify the
list-handling code to mirror static_stmt_from_create_call's behavior: when
enable_ui_source_map is true, append self.node_index_config_expr(span) to the
args passed for creating the list, and construct the same VarDecl/CallExpr
pattern (or delegate to static_stmt_from_create_call) so the resulting <list>
node includes the { nodeIndex } arg and produces a UISourceMapRecord; update the
snapshotCreateList path (and the similar block around lines 414-423) to either
call static_stmt_from_create_call with the appropriate element/callee and args,
or to perform the identical arg-append + VarDeclarator construction so
node-index/config handling is consistent.
In `@packages/webpack/react-webpack-plugin/src/loaders/main-thread.ts`:
- Around line 93-103: The code writes result.nodeIndexRecords into buildInfo but
TransformNodiffOutput defines uiSourceMapRecords instead; update the assignment
in the main-thread loader to use result.uiSourceMapRecords (and adjust any
typing if needed) so the property access matches the TransformNodiffOutput type;
specifically, change the value stored under NODE_INDEX_RECORDS_BUILD_INFO to
result.uiSourceMapRecords and ensure currentModule/_module buildInfo typing
remains compatible.
---
Outside diff comments:
In `@packages/webpack/react-webpack-plugin/src/loaders/options.ts`:
- Around line 166-176: The snapshot config uses the wrong key: replace the
incorrect snapshot property enableNodeIndex with enableUiSourceMap so the flag
matches the JsxTransformerConfig type and flows to the transform layer; locate
the snapshot object construction (the object containing preserveJsx, target,
runtimePkg) in the options loader (see the snapshot variable/block in
options.ts) and change enableNodeIndex ?? false to enableUiSourceMap ?? false,
keeping the rest of the logic intact so TypeScript strict mode and downstream
consumers (JsxTransformerConfig) see the correct flag.
---
Nitpick comments:
In @.github/rspeedy-core.instructions.md:
- Line 5: Clarify the cleanup guidance by naming the cleanup script/tool (e.g.,
the repo's clean/build script or CI job that runs cache cleanup), state whether
the note targets developers or automated tooling, and add a brief example
showing a tracked fixture file to preserve
(packages/rspeedy/core/test/<fixture>/node_modules/some-tracked-file.js) versus
generated artifacts to delete
(packages/rspeedy/core/test/<fixture>/node_modules/.cache/* or
node_modules/<pkg>-build-output), and also note that the LanguageTool flag for
"rspeedy" is a false positive and should be ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b21301b-01c3-4b04-a5a3-25f3f689df3b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
.changeset/soft-pumas-push.md.changeset/ten-lions-accept.md.github/react-transform.instructions.md.github/rspeedy-core.instructions.md.github/webpack-node-index.instructions.mdexamples/react-node-index/lynx.config.tsexamples/react-node-index/package.jsonexamples/react-node-index/src/App.cssexamples/react-node-index/src/App.tsxexamples/react-node-index/src/LazyPanel.tsxexamples/react-node-index/src/index.tsxexamples/react-node-index/src/rspeedy-env.d.tsexamples/react-node-index/tsconfig.jsonpackages/react/transform/__test__/fixture.spec.jspackages/react/transform/crates/swc_plugin_snapshot/lib.rspackages/react/transform/crates/swc_plugin_snapshot/napi.rspackages/react/transform/index.d.tspackages/react/transform/src/lib.rspackages/react/transform/swc-plugin-reactlynx/index.d.tspackages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.mdpackages/rspeedy/plugin-react/src/entry.tspackages/rspeedy/plugin-react/src/loaders.tspackages/rspeedy/plugin-react/src/pluginReactLynx.tspackages/rspeedy/plugin-react/test/config.test.tspackages/rspeedy/plugin-react/test/validate.test.tspackages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.mdpackages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.tspackages/webpack/react-webpack-plugin/src/loaders/main-thread.tspackages/webpack/react-webpack-plugin/src/loaders/options.tspackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/index.jsxpackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.jspackages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/test.config.cjspackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/src/LynxEncodePlugin.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
✅ Files skipped from review due to trivial changes (15)
- packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/index.jsx
- .changeset/soft-pumas-push.md
- examples/react-node-index/src/index.tsx
- examples/react-node-index/src/LazyPanel.tsx
- packages/rspeedy/plugin-react/test/validate.test.ts
- packages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.md
- examples/react-node-index/package.json
- .changeset/ten-lions-accept.md
- packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/test.config.cjs
- examples/react-node-index/src/rspeedy-env.d.ts
- examples/react-node-index/tsconfig.json
- packages/rspeedy/plugin-react/test/config.test.ts
- examples/react-node-index/src/App.css
- .github/webpack-node-index.instructions.md
- .github/react-transform.instructions.md
🚧 Files skipped from review as they are similar to previous changes (12)
- examples/react-node-index/src/App.tsx
- packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts
- packages/react/transform/swc-plugin-reactlynx/index.d.ts
- packages/rspeedy/plugin-react/src/loaders.ts
- packages/rspeedy/plugin-react/etc/react-rsbuild-plugin.api.md
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
- examples/react-node-index/lynx.config.ts
- packages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.md
- packages/webpack/react-webpack-plugin/test/cases/main-thread/lazy-bundle-sourcemap/rspack.config.js
- packages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.ts
- packages/react/transform/index.d.ts
upupming
changed the title
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
upupming
force-pushed
the
codex/node-index-asset
branch
from
ce657a0 to
806163f
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
upupming
force-pushed
the
codex/node-index-asset
branch
from
2f948d0 to
5812900
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f948d0775
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48fae34fa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
React External#368 Bundle Size — 591.76KiB (0%).cf55e95(current) vs b1ad1b9 main#353(baseline) Bundle metrics
|
| Current #368 |
Baseline #353 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch codex/node-index-asset Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b69867e13b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/webpack/template-webpack-plugin/src/LynxDebugMetadataPlugin.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a12b4d858d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
upupming
force-pushed
the
codex/node-index-asset
branch
from
a12b4d8 to
252f450
Compare
packages/webpack/react-webpack-plugin/src/loaders/main-thread.ts
Outdated
Show resolved
Hide resolved
upupming
force-pushed
the
codex/node-index-asset
branch
from
252f450 to
cf55e95
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf55e95925
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4 participants
Summary
beforeEncodecontext so custom plugins can attachdebugMetadataUrland related metadata before encodereact-node-indexexample that uploads the generated debug metadata and records git metadatanodeIndexfields for client compatibilityVerification
pnpm turbo buildpnpm vitest -ucargo test