refactor(devtools-vite): migrate from Babel to oxc-parser + MagicString#397
refactor(devtools-vite): migrate from Babel to oxc-parser + MagicString#397dake3601 wants to merge 3 commits intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughThe devtools-vite package migrates from Babel-based AST transformation to oxc-parser for parsing and MagicString for code injection. Dependencies are updated, core transformation modules are rewritten, and documentation reflects the tooling changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/devtools/skills/devtools-production/SKILL.md (1)
34-39:⚠️ Potential issue | 🟡 MinorAlign stripped-package docs with current implementation.
This section says
@tanstack/preact-devtoolsis auto-stripped, butpackages/devtools-vite/src/remove-devtools.ts(Line 6-9 in provided snippet) currently matches only React, core devtools, and Solid imports. Please either add preact handling inremove-devtools.tsor remove preact from this list to avoid incorrect production guidance.Suggested doc-only correction (if implementation is intentional)
- `@tanstack/react-devtools` -- `@tanstack/preact-devtools` - `@tanstack/solid-devtools` - `@tanstack/devtools`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools/skills/devtools-production/SKILL.md` around lines 34 - 39, The docs list includes `@tanstack/preact-devtools` but the implementation in remove-devtools.ts currently only matches React, core devtools, and Solid imports; either add preact handling to the matcher in remove-devtools.ts (e.g., include '@tanstack/preact-devtools' in the PACKAGES/regex used to detect imports and JSX removal in the remove logic) or remove preact from the SKILL.md list so docs reflect behavior—modify whichever side is intended so the list in SKILL.md and the import-matching array/regex in remove-devtools.ts stay in sync.
🧹 Nitpick comments (3)
packages/devtools-vite/src/inject-source.ts (1)
144-161: VariableDeclaration handling may miss deeply nested patterns.The code handles
const Foo = () => {}andconst Foo = function() {}but doesn't recursively check for more complex patterns likeconst Foo = memo(() => {})orconst Foo = forwardRef((props, ref) => {}). These wrapped components won't have source injection.This is a known limitation - supporting HOC-wrapped components would require more complex analysis. Consider documenting this limitation or adding a TODO for future enhancement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-vite/src/inject-source.ts` around lines 144 - 161, VariableDeclaration handling currently only checks decl.init for ArrowFunctionExpression or FunctionExpression, so HOC-wrapped patterns like memo(() => {}) or forwardRef((props, ref) => {}) are missed; update the VariableDeclaration branch (where decl.init is inspected) to unwrap CallExpression wrappers by checking if decl.init.type === 'CallExpression' and then examine its arguments (recursively if needed) for ArrowFunctionExpression or FunctionExpression before calling processFunction, or if you don't want to implement unwrapping now, add a clear TODO comment and a short documentation note near processFunction/VariableDeclaration explaining this limitation and linking to a future task to implement recursive CallExpression unwrapping.packages/devtools-vite/src/remove-devtools.ts (1)
162-172: Import reconstruction may lose original formatting.When rebuilding import statements with remaining specifiers (line 171), the reconstruction creates a normalized format that may differ from the original (e.g., loses type imports, multiline formatting, or trailing commas). This is acceptable for production builds but worth noting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-vite/src/remove-devtools.ts` around lines 162 - 172, The current overwrite reconstructs imports via `specTexts.join` which normalizes formatting and can drop `type` modifiers, trailing commas, and multiline layout; instead preserve the original import text by using the AST ranges and original code slices to only remove the ranges for deleted specifiers: compute the removal ranges from `node.specifiers` vs `remaining`, splice those ranges out of `code.slice(node.start, node.end)` (or use `s.remove` on each removed specifier range) and then `s.overwrite` or `s.appendRight` the resulting original-import string—this keeps `importKind`, whitespace, comments, trailing commas and multiline formatting intact while still removing the unwanted specifiers (refer to `remaining`, `specTexts`, `node`, and `s.overwrite` in the diff).packages/devtools-vite/src/enhance-logs.ts (1)
51-57: Parenthesis scan could fail on unusual formatting.The scan from
callee.endto find(works for typicalconsole.log(...)calls but could fail if there's a comment between the callee and the parenthesis (e.g.,console.log /* comment */ (...)). While rare in practice, consider using the CallExpression's argument span instead.🔧 Alternative approach using CallExpression structure
If oxc-parser provides the arguments array with position info, you could use:
- // Find the opening '(' of the call by scanning forward from callee end - let parenOffset = callee.end - while (parenOffset < code.length && code[parenOffset] !== '(') { - parenOffset++ - } - // Insert right after '(' - s.appendRight(parenOffset + 1, spreadStr) + // Insert at the start of the first argument, or right after callee if no args + const firstArg = node.arguments[0] + if (firstArg) { + s.appendRight(firstArg.start, spreadStr) + } else { + // No arguments: console.log() -> console.log(spreadStr) + s.appendRight(node.end - 1, spreadStr) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-vite/src/enhance-logs.ts` around lines 51 - 57, The current scan from callee.end to find '(' (using parenOffset and s.appendRight) can fail with comments or weird whitespace between callee and the paren; instead compute the insertion point from the CallExpression node's arguments: if node.arguments.length > 0 use node.arguments[0].start to insert spreadStr before the first argument, otherwise fall back to using the CallExpression's end/span (e.g., search between callee.end and node.end for the '(' or insert just before node.end-1) so you avoid relying solely on callee.end and make the insertion robust; update references to parenOffset, callee.end, s.appendRight and spreadStr accordingly.
🤖 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/devtools-vite/package.json`:
- Around line 63-64: The dependency oxc-parser is pinned at ^0.72.0 and should
be reviewed and updated; open packages/devtools-vite/package.json, evaluate
compatibility of oxc-parser (symbol: "oxc-parser") against the current latest
(≈0.120.0), run the package upgrade (e.g., npm/yarn/pnpm) to a newer safe
version, run the test suite and local build to catch breaking changes, and if
you must keep 0.72.0 document the compatibility rationale beside the dependency
(or in CHANGELOG/README) so future reviewers know why it is pinned; also
consider aligning or noting magic-string (symbol: "magic-string") remains
current.
In `@packages/devtools-vite/src/inject-plugin.ts`:
- Around line 8-14: The devtools package list is duplicated and inconsistent
(see devtoolsPackages in inject-plugin.ts vs checks in remove-devtools.ts);
extract a single exported constant (e.g. TANSTACK_DEVTOOLS_PACKAGES) from a new
constants module and replace the local devtoolsPackages array in
inject-plugin.ts and any package checks in remove-devtools.ts to import and use
TANSTACK_DEVTOOLS_PACKAGES so both modules share the exact same list.
In `@packages/devtools-vite/src/offset-to-loc.ts`:
- Around line 9-37: The current createLocMapper uses JS string code-unit
indexing (charCodeAt/length) but callers pass oxc-parser byte offsets, causing
wrong positions for non-ASCII text; update the call sites (e.g., where
enhance-logs.ts calls createLocMapper) to convert oxc byte offsets to JS
code-unit indices before calling createLocMapper OR modify createLocMapper to
accept byte offsets by first mapping byte offset → code-unit index (decode UTF-8
byte positions into JS string indices) and then perform the existing binary
search; finally add a regression test that feeds source with multi-byte
characters (emoji/CJK) and verifies line/column results from createLocMapper
match expected positions to prevent regressions.
In `@packages/devtools-vite/src/remove-devtools.ts`:
- Around line 6-9: The function isTanStackDevtoolsImport currently only checks
'@tanstack/react-devtools', '@tanstack/devtools', and
'@tanstack/solid-devtools', so imports of '@tanstack/preact-devtools' and
'@tanstack/vue-devtools' are not detected and thus not removed; update
isTanStackDevtoolsImport to include '@tanstack/preact-devtools' and
'@tanstack/vue-devtools' (the same package names listed in devtoolPackages in
plugin.ts) so those imports are properly recognized and stripped during
production builds.
---
Outside diff comments:
In `@packages/devtools/skills/devtools-production/SKILL.md`:
- Around line 34-39: The docs list includes `@tanstack/preact-devtools` but the
implementation in remove-devtools.ts currently only matches React, core
devtools, and Solid imports; either add preact handling to the matcher in
remove-devtools.ts (e.g., include '@tanstack/preact-devtools' in the
PACKAGES/regex used to detect imports and JSX removal in the remove logic) or
remove preact from the SKILL.md list so docs reflect behavior—modify whichever
side is intended so the list in SKILL.md and the import-matching array/regex in
remove-devtools.ts stay in sync.
---
Nitpick comments:
In `@packages/devtools-vite/src/enhance-logs.ts`:
- Around line 51-57: The current scan from callee.end to find '(' (using
parenOffset and s.appendRight) can fail with comments or weird whitespace
between callee and the paren; instead compute the insertion point from the
CallExpression node's arguments: if node.arguments.length > 0 use
node.arguments[0].start to insert spreadStr before the first argument, otherwise
fall back to using the CallExpression's end/span (e.g., search between
callee.end and node.end for the '(' or insert just before node.end-1) so you
avoid relying solely on callee.end and make the insertion robust; update
references to parenOffset, callee.end, s.appendRight and spreadStr accordingly.
In `@packages/devtools-vite/src/inject-source.ts`:
- Around line 144-161: VariableDeclaration handling currently only checks
decl.init for ArrowFunctionExpression or FunctionExpression, so HOC-wrapped
patterns like memo(() => {}) or forwardRef((props, ref) => {}) are missed;
update the VariableDeclaration branch (where decl.init is inspected) to unwrap
CallExpression wrappers by checking if decl.init.type === 'CallExpression' and
then examine its arguments (recursively if needed) for ArrowFunctionExpression
or FunctionExpression before calling processFunction, or if you don't want to
implement unwrapping now, add a clear TODO comment and a short documentation
note near processFunction/VariableDeclaration explaining this limitation and
linking to a future task to implement recursive CallExpression unwrapping.
In `@packages/devtools-vite/src/remove-devtools.ts`:
- Around line 162-172: The current overwrite reconstructs imports via
`specTexts.join` which normalizes formatting and can drop `type` modifiers,
trailing commas, and multiline layout; instead preserve the original import text
by using the AST ranges and original code slices to only remove the ranges for
deleted specifiers: compute the removal ranges from `node.specifiers` vs
`remaining`, splice those ranges out of `code.slice(node.start, node.end)` (or
use `s.remove` on each removed specifier range) and then `s.overwrite` or
`s.appendRight` the resulting original-import string—this keeps `importKind`,
whitespace, comments, trailing commas and multiline formatting intact while
still removing the unwanted specifiers (refer to `remaining`, `specTexts`,
`node`, and `s.overwrite` in the diff).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efb58404-8540-40fe-a2a1-4521664de5be
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.changeset/common-bottles-move.md_artifacts/domain_map.yamldocs/architecture.mddocs/source-inspector.mddocs/vite-plugin.mdpackages/devtools-vite/package.jsonpackages/devtools-vite/skills/devtools-vite-plugin/SKILL.mdpackages/devtools-vite/skills/devtools-vite-plugin/references/vite-options.mdpackages/devtools-vite/src/ast-utils.tspackages/devtools-vite/src/babel.tspackages/devtools-vite/src/enhance-logs.tspackages/devtools-vite/src/inject-plugin.test.tspackages/devtools-vite/src/inject-plugin.tspackages/devtools-vite/src/inject-source.test.tspackages/devtools-vite/src/inject-source.tspackages/devtools-vite/src/offset-to-loc.tspackages/devtools-vite/src/remove-devtools.test.tspackages/devtools-vite/src/remove-devtools.tspackages/devtools/skills/devtools-marketplace/SKILL.mdpackages/devtools/skills/devtools-production/SKILL.md
💤 Files with no reviewable changes (1)
- packages/devtools-vite/src/babel.ts
| "magic-string": "^0.30.0", | ||
| "oxc-parser": "^0.72.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest versions and security advisories for new dependencies
echo "=== Checking magic-string ==="
curl -s https://registry.npmjs.org/magic-string/latest | jq '.version'
echo "=== Checking oxc-parser ==="
curl -s https://registry.npmjs.org/oxc-parser/latest | jq '.version'
# Check if 0.72.0 exists for oxc-parser
echo "=== Checking oxc-parser@0.72.0 exists ==="
curl -s https://registry.npmjs.org/oxc-parser/0.72.0 | jq '.version // "NOT FOUND"'Repository: TanStack/devtools
Length of output: 189
Consider updating oxc-parser to a more recent version.
While magic-string@^0.30.0 is current (latest is 0.30.21), oxc-parser@0.72.0 is significantly outdated; the latest version is 0.120.0. Check the changelog between 0.72.0 and a more recent release to ensure no critical fixes, breaking changes, or performance improvements are missed. If 0.72.0 is intentionally pinned for compatibility, document the rationale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/devtools-vite/package.json` around lines 63 - 64, The dependency
oxc-parser is pinned at ^0.72.0 and should be reviewed and updated; open
packages/devtools-vite/package.json, evaluate compatibility of oxc-parser
(symbol: "oxc-parser") against the current latest (≈0.120.0), run the package
upgrade (e.g., npm/yarn/pnpm) to a newer safe version, run the test suite and
local build to catch breaking changes, and if you must keep 0.72.0 document the
compatibility rationale beside the dependency (or in CHANGELOG/README) so future
reviewers know why it is pinned; also consider aligning or noting magic-string
(symbol: "magic-string") remains current.
| const devtoolsPackages = [ | ||
| '@tanstack/react-devtools', | ||
| '@tanstack/solid-devtools', | ||
| '@tanstack/vue-devtools', | ||
| '@tanstack/svelte-devtools', | ||
| '@tanstack/angular-devtools', | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare devtools package lists across files
echo "=== inject-plugin.ts packages ==="
rg -A8 'devtoolsPackages\s*=' packages/devtools-vite/src/inject-plugin.ts
echo ""
echo "=== remove-devtools.ts packages ==="
rg -A5 'isTanStackDevtoolsImport' packages/devtools-vite/src/remove-devtools.ts
echo ""
echo "=== plugin.ts packages ==="
rg -A8 'devtoolPackages\s*=' packages/devtools-vite/src/plugin.tsRepository: TanStack/devtools
Length of output: 1230
Inconsistent devtools package handling across plugin files.
Different files define or check for different devtools packages: inject-plugin.ts includes svelte and angular (plus react, solid, vue), while remove-devtools.ts only checks for react, solid, and devtools. This inconsistency could cause issues if one file is updated to support a new framework but the other isn't. Extracting a shared constant would ensure consistency.
🛠️ Suggested approach: create shared constant
Create a shared constants file:
// packages/devtools-vite/src/constants.ts
export const TANSTACK_DEVTOOLS_PACKAGES = [
'@tanstack/react-devtools',
'@tanstack/preact-devtools',
'@tanstack/solid-devtools',
'@tanstack/vue-devtools',
'@tanstack/svelte-devtools',
'@tanstack/angular-devtools',
'@tanstack/devtools',
] as constThen import and use in both remove-devtools.ts and inject-plugin.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/devtools-vite/src/inject-plugin.ts` around lines 8 - 14, The
devtools package list is duplicated and inconsistent (see devtoolsPackages in
inject-plugin.ts vs checks in remove-devtools.ts); extract a single exported
constant (e.g. TANSTACK_DEVTOOLS_PACKAGES) from a new constants module and
replace the local devtoolsPackages array in inject-plugin.ts and any package
checks in remove-devtools.ts to import and use TANSTACK_DEVTOOLS_PACKAGES so
both modules share the exact same list.
| * Pre-compute line start offsets for fast byte-offset to line/column conversion. | ||
| * Returns a function that maps a byte offset to `{ line, column }`. | ||
| * | ||
| * Build: O(n) single pass over the source string. | ||
| * Lookup: O(log n) binary search. | ||
| */ | ||
| export function createLocMapper(source: string): (offset: number) => Loc { | ||
| const lineStarts: Array<number> = [0] | ||
| for (let i = 0; i < source.length; i++) { | ||
| if (source.charCodeAt(i) === 10 /* \n */) { | ||
| lineStarts.push(i + 1) | ||
| } | ||
| } | ||
|
|
||
| return (offset: number): Loc => { | ||
| let lo = 0 | ||
| let hi = lineStarts.length - 1 | ||
| while (lo < hi) { | ||
| const mid = (lo + hi + 1) >> 1 | ||
| if (lineStarts[mid]! <= offset) { | ||
| lo = mid | ||
| } else { | ||
| hi = mid - 1 | ||
| } | ||
| } | ||
| return { | ||
| line: lo + 1, | ||
| column: offset - lineStarts[lo]!, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In oxc-parserv0.72.0 JavaScript bindings, areNode.startandNode.end measured as UTF-8 byte offsets or UTF-16 code-unit indices?
💡 Result:
In oxc-parser v0.72.0 JavaScript bindings, Node.start and Node.end are measured as UTF-8 byte offsets.
Citations:
- 1: parser,codegen: comment attachment and print all comments oxc-project/oxc#19671
- 2: feat(parser): utf16 spans oxc-project/oxc#959
- 3: https://crates.io/crates/oxc_span
- 4: AST nodes with loc and/or range fields. oxc-project/oxc#10307
- 5: unicode characters result in incorrect start/end values oxc-project/oxc#7508
🏁 Script executed:
#!/bin/bash
# Read-only verification: inspect call sites and offset sources.
rg -nP --type=ts -C3 '\bcreateLocMapper\s*\('
rg -nP --type=ts -C3 '\bnode\.start\b|\bnode\.end\b'Repository: TanStack/devtools
Length of output: 5877
Critical: offset-unit mismatch causes incorrect locations for non-ASCII source files.
The implementation uses UTF-16 code-unit indexing (via source.charCodeAt(), source.length, string offset arithmetic), but oxc-parser byte offsets are passed directly at call sites (e.g., enhance-logs.ts:41). For ASCII text this works by coincidence; for non-ASCII characters (emoji, multi-byte UTF-8), byte offsets and code-unit offsets diverge, producing wrong line/column mappings.
Convert oxc-parser byte offsets to JavaScript string indices before passing to createLocMapper, or add byte-to-code-unit conversion within the mapper itself. Add a regression test with non-ASCII characters (e.g., emoji, CJK) to prevent silent data corruption.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/devtools-vite/src/offset-to-loc.ts` around lines 9 - 37, The current
createLocMapper uses JS string code-unit indexing (charCodeAt/length) but
callers pass oxc-parser byte offsets, causing wrong positions for non-ASCII
text; update the call sites (e.g., where enhance-logs.ts calls createLocMapper)
to convert oxc byte offsets to JS code-unit indices before calling
createLocMapper OR modify createLocMapper to accept byte offsets by first
mapping byte offset → code-unit index (decode UTF-8 byte positions into JS
string indices) and then perform the existing binary search; finally add a
regression test that feeds source with multi-byte characters (emoji/CJK) and
verifies line/column results from createLocMapper match expected positions to
prevent regressions.
| const isTanStackDevtoolsImport = (source: string) => | ||
| source === '@tanstack/react-devtools' || | ||
| source === '@tanstack/devtools' || | ||
| source === '@tanstack/solid-devtools' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the devtools packages list in plugin.ts for consistency
echo "=== Packages in plugin.ts transform hook ==="
rg -A10 'devtoolPackages\s*=' packages/devtools-vite/src/plugin.ts
echo ""
echo "=== All tanstack devtools package references ==="
rg '@tanstack/(react|preact|solid|vue|angular)-devtools|@tanstack/devtools' packages/devtools-vite/src/Repository: TanStack/devtools
Length of output: 12965
🏁 Script executed:
rg -B5 -A5 'preact-devtools|vue-devtools' packages/devtools-vite/src/remove-devtools.test.tsRepository: TanStack/devtools
Length of output: 43
🏁 Script executed:
head -50 packages/devtools-vite/src/remove-devtools.test.tsRepository: TanStack/devtools
Length of output: 1415
Add missing TanStack devtools packages to the removal check.
isTanStackDevtoolsImport omits @tanstack/preact-devtools and @tanstack/vue-devtools, which are included in the devtoolPackages array in plugin.ts. This causes these imports to bypass removal during production builds.
🐛 Proposed fix
const isTanStackDevtoolsImport = (source: string) =>
source === '@tanstack/react-devtools' ||
source === '@tanstack/devtools' ||
- source === '@tanstack/solid-devtools'
+ source === '@tanstack/solid-devtools' ||
+ source === '@tanstack/preact-devtools' ||
+ source === '@tanstack/vue-devtools'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/devtools-vite/src/remove-devtools.ts` around lines 6 - 9, The
function isTanStackDevtoolsImport currently only checks
'@tanstack/react-devtools', '@tanstack/devtools', and
'@tanstack/solid-devtools', so imports of '@tanstack/preact-devtools' and
'@tanstack/vue-devtools' are not detected and thus not removed; update
isTanStackDevtoolsImport to include '@tanstack/preact-devtools' and
'@tanstack/vue-devtools' (the same package names listed in devtoolPackages in
plugin.ts) so those imports are properly recognized and stripped during
production builds.
🎯 Changes
Replace Babel toolchain with oxc-parser for parsing and MagicString for source transformation in devtools-vite.
This improves vite dev performance. Having the plugin in our vite config increase inner loop with ~20 sec with these changes it drops down to around ~7 in my machine.
✅ Checklist
pnpm test:pr.🚀 Release Impact
Summary by CodeRabbit
Documentation
Chores