fix(live): scan JSX comment variant markers; never whole-file-inject raw JSX (#454) - #455
fix(live): scan JSX comment variant markers; never whole-file-inject raw JSX (#454)#455kticoder wants to merge 2 commits into
Conversation
…SX source
injectVariantsFromSource only scanned for HTML comment markers
(<!-- impeccable-variants-start -->), but JSX/TSX wrappers necessarily
carry JSX comment markers ({/* ... */}) since HTML comments are invalid
inside a JSX element tree. The scan therefore always missed on React
targets and fell through to whole-file injection, rendering raw JSX
({expressions}, marker text) as literal page content.
- Scan both marker syntaxes before giving up.
- For .jsx/.tsx sources with no markers, treat as missing wrapper
(recovery/orphan path) instead of injecting the entire raw file.
- Strip JSX comments in normalizeSourceFallbackBlock so they do not
render as text in the DOMParser preview.
Fixes pbakaus#454
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const markerPairs = [ | ||
| { open: '<!--', close: '-->' }, | ||
| { open: '{/*', close: '*/}' }, | ||
| ]; |
There was a problem hiding this comment.
The new HTML/JSX marker extraction, markerless JSX/TSX skip, markerless HTML fallback, and JSX-comment normalization paths have no behavioral regression coverage. The focused suite remains green after each behavior is removed, so a future change can restore raw JSX injection or corrupt fallback previews without detection. Add focused inputs for both marker syntaxes, markerless .jsx and .tsx files, markerless HTML, and JSX comments before parsing.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
Implementation diff without matching test changes
- Compared the parent and PR revisions for the changed browser source and focused test file; it shows the new marker/fallback/comment-removal code and no test-file change, confirming the coverage gap.
Focused live-browser source suite passing on the PR revision
- Ran `node --test tests/live-browser-source.test.mjs` in `/home/user/repo`; all 8 tests passed, confirming the existing narrow suite is green but does not establish coverage of the new branches.
Focused suite passes after each changed fallback behavior is removed
- Created temporary copies outside the repository, removed JSX marker extraction, markerless JSX skip, markerless HTML fallback, and JSX comment stripping one at a time, and ran the focused suite; every mutation passed 8/8, proving these behaviors lack regression detection.
Parent revision leaves a JSX comment in normalized source
- Executed the focused normalization harness against `832b3742^`; JSX comment text remained in the output, establishing the pre-change behavior.
PR revision removes a JSX comment while preserving HTML input
- Executed the focused normalization harness against the PR revision; JSX comment text was removed and equivalent HTML was unchanged, demonstrating the changed behavior.
Search results for direct regression coverage of changed fallback paths
- Searched all test files for the new helper, marker-pair parsing, markerless JSX warning, and JSX marker syntax; results contain only existing generic source-contract references, not representative behavioral cases.
Greptile SummaryImproves JSX/TSX live-mode recovery by adopting framework-rendered variant wrappers after reloads, recognizing JSX comment markers, creating the missing wrapper around extracted JSX marker content, and preventing raw markerless JSX from being inserted into the page. Runtime checks exercised all affected recovery paths: JSX marker blocks resumed cycling and advanced to the second variant without rendering marker text; markerless JSX did not create a wrapper or display raw JSX; and an existing live wrapper remained intact without a source request. Confidence Score: 3/5
T-Rex LogsWhat T-Rex did
Reviews (3): Last reviewed commit: "fix(live): adopt live DOM wrapper for JS..." | Re-trigger Greptile |
… around JSX marker block
The first pass fixed marker syntax but exposed two follow-ups on real
React targets:
1. JSX wrappers carry their markers INSIDE the wrapper div (live-wrap.mjs
JSX branch), so the extracted block lacked the wrapper element and
resume logged 'Variant wrapper not found in source file', stranding
the bar. Synthesize the wrapper element around the extracted block.
2. On a Vite full reload React has already re-rendered the wrapper with
evaluated expressions. Replacing it with a DOMParser clone of raw
source would render {expressions} literally and detach React from its
nodes. Adopt the live DOM wrapper instead: count variants, restore the
visible index, and resume CYCLING without touching the tree.
| if (arrivedVariants <= 0) { | ||
| if (state === 'GENERATING' && !opts.generationCompleted) return; | ||
| recoverEmptyCycling('jsx-dom-adopt-empty'); | ||
| return; |
There was a problem hiding this comment.
Empty JSX adopt skips retries
High Severity
When a JSX live wrapper is present but still has zero variants after generationCompleted, the adopt path calls recoverEmptyCycling immediately. The source-fallback path below retries several times for the same stale-scaffold case, so a wrap that HMR’d before variants can tear the session down instead of waiting for Fast Refresh.
Reviewed by Cursor Bugbot for commit 88d501c. Configure here.
| } else { | ||
| block = html; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fetch path still replaces React
High Severity
The new JSX adopt guard only runs synchronously before fetch. After the source download, an existingWrapper that appeared via HMR is still replaceChild’d with a DOMParser clone—the same React detach / literal {expression} failure #454 describes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 88d501c. Configure here.
| } else { | ||
| block = html; | ||
| } | ||
| } |
There was a problem hiding this comment.
JSX no-marker skips recovery
Medium Severity
For .jsx/.tsx with no markers, block is set to '' so the wrapper lookup fails. That branch only discards when orphanDiscard is set; the generationCompleted done-fallback caller just returns, leaving the tab stuck in GENERATING instead of recovering.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 88d501c. Configure here.
github-actions
Bot
added
blocked: review threads
|
Thanks for taking the time to contribute. This PR links issue #454, but I could not find explicit authorization from @pbakaus or @abdulwahabone for this implementation. This repository requires maintainer approval before implementation, so I’m closing the PR for now. This check was performed automatically by Codex; if this classification is mistaken, a maintainer can correct it. |
|
Reopening after approval |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 88d501c. Configure here.
| // JSX wrappers carry their markers INSIDE the wrapper div, so the | ||
| // extracted block lacks the wrapper element; synthesize it (#454). | ||
| block = '<div data-impeccable-variants="' + sessionId + '" style="display: contents">' + block + '</div>'; | ||
| } |
There was a problem hiding this comment.
Synthesized wrapper drops attributes
Medium Severity
When JSX markers match, the rebuilt wrapper only sets data-impeccable-variants and style. The real open tag outside those markers also carries data-impeccable-variant-count and, for insert sessions, data-impeccable-mode="insert", so source-fallback injection loses planned count and insert-mode behavior.
Reviewed by Cursor Bugbot for commit 88d501c. Configure here.
|
Thanks for the PR. Impeccable is moving quickly, and this PR is currently waiting on contributor action. It has been waiting for contributor action for 7 days. Please address the outstanding review feedback, draft state, or explicit maintainer wait request. PRs that are still waiting on contributor action after 14 days are closed automatically. If nothing changes, this PR may be closed on or after 2026-08-17. Happy to reopen when it is ready to continue. |
4 participants
Fixes #454.
Problem
injectVariantsFromSource()scans only for HTML comment markers, butlive-wrap.mjsnecessarily emits JSX comment markers ({/* impeccable-variants-start ID */}) inside JSX/TSX wrappers (line ~322; an HTML comment is invalid in a JSX element tree). On React targets the scan always misses, so the no-HMR fallback injects the entire raw source file, rendering{expressions}and marker text as literal page content.Fix (kept minimal)
<!-- -->and{/* */}) before falling back..jsx/.tsxsources with no markers, treat it like a missing wrapper (existing recovery/orphan path) instead of whole-file injection — raw JSX can never render correctly, so recovery is strictly better than showing source text. HTML behavior is unchanged.normalizeSourceFallbackBlock: strip JSX comments so{/* Original */}etc. don't render as text in the DOMParser preview.Testing
node --checkpasses..tsxcovered.{liste.length}on screen) before the patch; described in [Bug] Live mode: injectVariantsFromSource renders raw JSX as text for React targets (JSX comment markers never match HTML comment scan) #454.Not run:
bun run build/ live-e2e (no bun locally) — happy to iterate if CI flags the generated outputs.🤖 Generated with Claude Code
Note
Medium Risk
Changes the live cycling injection path for JSX/React, but behavior is narrowly scoped to marker parsing and adoption with existing recovery retries preserved.
Overview
Fixes React/JSX live variant injection when the no-HMR source fallback runs: the browser no longer treats raw
.jsx/.tsxas injectable HTML or misses JSX-only variant markers.For JSX/TSX,
injectVariantsFromSourcenow adopts an existing live DOM wrapper (after Vite reload / Fast Refresh) instead of cloning aDOMParserpreview, which previously showed{expressions}as literal text and broke React’s tree (#454).When fetching source, variant blocks are found via both
<!-- ... -->and{/* ... */}markers; JSX blocks get a synthesized wrapper when markers sit inside the element tree. Unmarked JSX skips whole-file injection and uses the existing empty/orphan recovery path; HTML unmarked behavior is unchanged.normalizeSourceFallbackBlockalso strips JSX comments from DOMParser previews so comment markers do not render as visible text.Reviewed by Cursor Bugbot for commit 88d501c. Bugbot is set up for automated code reviews on this repo. Configure here.