Skip to content

fix(devtools-vite): preserve valid syntax when removing devtools JSX#480

Open
vedant416 wants to merge 2 commits into
TanStack:mainfrom
vedant416:fix/remove-devtools
Open

fix(devtools-vite): preserve valid syntax when removing devtools JSX#480
vedant416 wants to merge 2 commits into
TanStack:mainfrom
vedant416:fix/remove-devtools

Conversation

@vedant416

@vedant416 vedant416 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🎯 Changes

Fixes #457

Issue Summary

  • Devtools JSX (<TanStackDevtools />) was being fully removed without taking into account the context in which it was being used.
  • This could lead to invalid syntax being left behind, which would break builds.
  • Below is the example of bug:
// before removal
const devtools = <TanStackDevtools />

// after removal (broken)
const devtools =

Fix

  • Devtools JSX nodes are now only fully removed when their parent is a JSXElement or JSXFragment.
  • In every other context, the node is replaced with null instead, which is always valid as an expression.
  • New changes preserve existing behavior and fix the above issue.
  • All the existing testcases pass and I have also added testcases for the new behavior.
  • Below are some examples of the new behavior:
// before removal:
const devtools0 = <div><TanStackDevtools /></div>
const devtools1 = <TanStackDevtools />
const devtools2 = functionCall(<TanStackDevtools />)
const devtools3 = true ? <TanStackDevtools /> : <>fallback</>
const devtools4 = {
  devtools: <TanStackDevtools />
}
const devtools5 = (<div>
    {<TanStackDevtools plugins={[]} />}
</div>)

// after removal:
const devtools0 = <div></div>
const devtools1 = null
const devtools2 = functionCall(null)
const devtools3 = true ? null : <>fallback</>
const devtools4 = {
  devtools: null
}
const devtools5 = (<div>
    {null}
</div>)

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm test:pr.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

  • Bug Fixes
    • Improved devtools removal so generated code stays valid in more cases, including parenthesized expressions and JSX contexts.
    • Nested devtools content inside JSX is now removed cleanly, leaving the surrounding UI intact.
    • Added broader test coverage for common JavaScript and JSX patterns to prevent regressions.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 32f77c38-71b8-4e25-b4ab-8cd02a3c7004

📥 Commits

Reviewing files that changed from the base of the PR and between 92f69d0 and 7196ae7.

📒 Files selected for processing (3)
  • .changeset/happy-clubs-wave.md
  • packages/devtools-vite/src/remove-devtools.test.ts
  • packages/devtools-vite/src/remove-devtools.ts

📝 Walkthrough

Walkthrough

The removeDevtools transform's replacement logic is changed: devtools nodes nested inside JSXElement or JSXFragment are now fully removed, while nodes in other expression contexts are replaced with null instead of removed outright. Tests are updated/extended accordingly, and a changeset documents the fix.

Changes

Devtools removal syntax fix

Layer / File(s) Summary
Removal logic update
packages/devtools-vite/src/remove-devtools.ts
Changes replacement behavior from "ParenthesizedExpression → null, else remove" to "nested in JSX → remove, else replace with null", with an added explanatory comment.
Test coverage and changeset
packages/devtools-vite/src/remove-devtools.test.ts, .changeset/happy-clubs-wave.md
Updates the parenthesized return test to expect null replacement, adds tests for null replacement across multiple expression forms and full JSX subtree removal, and adds a minor changeset entry.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • TanStack/devtools#449: Both PRs modify the same remove-devtools.ts replacement logic and its tests for preserving valid syntax when removing devtools nodes.

Suggested reviewers: AlemTuzlak

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preserving valid syntax when removing devtools JSX.
Description check ✅ Passed The description follows the template and covers changes, checklist, and release impact with sufficient detail.
Linked Issues check ✅ Passed The code changes and tests directly address issue #457 by removing devtools safely without breaking syntax.
Out of Scope Changes check ✅ Passed The changes are limited to the bug fix, tests, and changeset, with no obvious unrelated edits.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Devtools vite plugin causes bug.

1 participant