Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@react-grab/cli
grab
@react-grab/ami
@react-grab/amp
@react-grab/claude-code
@react-grab/codex
@react-grab/cursor
@react-grab/droid
@react-grab/gemini
@react-grab/opencode
react-grab
@react-grab/relay
@react-grab/utils
commit: |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
| ); | ||
| } | ||
|
|
||
| const grouped = Map.groupBy(countByComponent, ([, count]) => count); |
There was a problem hiding this comment.
Critical: Browser Compatibility Issue
Map.groupBy() is an ES2024 feature with very limited browser support (not available in Safari, Firefox ESR, and older Chrome versions). This will cause runtime errors in unsupported browsers.
Recommended fix:
const grouped = new Map<number, Array<[string, number]>>();
for (const [name, count] of countByComponent) {
if (!grouped.has(count)) {
grouped.set(count, []);
}
grouped.get(count)!.push([name, count]);
}
packages/cli/src/utils/transform.ts
Outdated
|
|
||
| const VITE_REMOVAL_PATTERNS: RegExp[] = [ | ||
| /\s*import\s*\(\s*["']react-scan["']\s*\);?/g, | ||
| /\s*import\s*\(\s*["']react-scan["']\s*\)\s*\.then\s*\([^)]*\)[^;]*;?/g, |
There was a problem hiding this comment.
Risk: Overly Greedy Pattern
The pattern [^)]* followed by [^;]* is too permissive and could accidentally match beyond the intended .then() call. For example:
import("react-scan").then(() => console.log("loaded")); doSomething();The [^;]* will match ) doSomething(), potentially removing user code after the import.
Consider making the pattern more specific:
/\s*import\s*\(\s*["']react-scan["']\s*\)\s*\.then\s*\([^)]*\)(?:\s*\.catch\s*\([^)]*\))?;?/g
packages/cli/src/utils/transform.ts
Outdated
| const WEBPACK_REMOVAL_PATTERNS: RegExp[] = [ | ||
| /\s*import\s*\(\s*["']react-scan["']\s*\);?/g, | ||
| /^\s*import\s+(?:\{[^}]*\}|\*\s+as\s+\w+|\w+)\s+from\s+["']react-scan["'];?\s*$/gm, | ||
| /if\s*\(\s*process\.env\.NODE_ENV\s*===\s*["']development["']\s*\)\s*\{\s*\}/g, |
There was a problem hiding this comment.
Minor: Pattern May Not Match After First Removal
This pattern matches empty if blocks, but it relies on \s* to match any whitespace between { and }. After the first pattern removes the import statement, there could be varied whitespace (newlines, indentation) that should be matched. The pattern should work, but consider testing edge cases with different indentation styles.
Also, this pattern could theoretically match legitimate empty development conditionals unrelated to React Scan. Consider adding a comment in the code to document this behavior.
packages/cli/src/commands/migrate.ts
Outdated
| const combinedOriginal = removalResult.originalContent!; | ||
| const combinedNew = addResult.newContent!; |
There was a problem hiding this comment.
Bug: Incorrect Preview Diff
When removal and addition happen in the same file, this shows removalResult.originalContent → addResult.newContent. However, addResult was calculated from the original file (line 275-281), not from the file after React Scan removal. This means addResult.newContent still contains React Scan code, making the preview diff misleading.
The actual application order (lines 389-401) is correct, so this is a display-only bug.
Fix:
let combinedNew = removalResult.newContent!;
// Need to recalculate addResult based on removed content
// Or chain the operations properlyAlternatively, you could apply the removal transform to addResult.newContent before showing the diff, or show both diffs separately even when they're in the same file.
| ]; | ||
|
|
||
| export const REACT_SCAN_DETECTION_PATTERNS: RegExp[] = [ | ||
| /["'`][^"'`]*react-scan/, |
There was a problem hiding this comment.
Good: Comprehensive Pattern Detection
The detection patterns cover multiple installation methods (CDN, npm, import, require). However, pattern /["'\][^"'\`]*react-scan/` could theoretically match "react-scan" in comments or strings unrelated to the actual library.
This is unlikely to cause issues in practice, but worth noting for future refinement if false positives occur.
|
|
||
| expect(result.success).toBe(true); | ||
| expect(result.noChanges).toBeFalsy(); | ||
| expect(result.newContent).not.toContain("react-scan"); |
There was a problem hiding this comment.
Test Gap: Missing Assertion
The test verifies react-scan is removed and React is preserved, but doesn't verify that the empty if block itself was also removed. Consider adding:
expect(result.newContent).not.toContain('if (process.env.NODE_ENV');|
|
||
| export const startRecording = (): void => { | ||
| if (!setupInstrumentation()) return; | ||
| renderLogHistory = []; |
There was a problem hiding this comment.
Minor: Potential Data Loss
startRecording() clears renderLogHistory unconditionally. If a user accidentally clicks record twice or the function is called programmatically multiple times, they lose their previous recording data without warning.
Consider either:
- Checking if there's existing data and prompting the user
- Not clearing the history automatically
- Documenting this behavior clearly in the UI
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Bugbot Autofix prepared fixes for 4 of the 4 bugs found in the latest run.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 17 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/cli/test/detect.test.ts">
<violation number="1" location="packages/cli/test/detect.test.ts:229">
P2: These tests use a global mock that returns `package.json` content for all file reads. Since `detectReactScan` scans multiple files (like `layout.tsx`, `index.html`) using regex patterns, and the mocked `package.json` contains "react-scan", the function will incorrectly "detect" `react-scan` in every checked file.
This creates a false positive scenario where `detectedFiles` is populated even when checking only dependencies, potentially masking bugs in the file detection logic.
Fix by strictly mocking `package.json` and other files separately.</violation>
</file>
<file name="packages/cli/src/utils/transform.ts">
<violation number="1" location="packages/cli/src/utils/transform.ts:1633">
P1: This regex strictly requires parentheses around the component in the conditional expression. If the code was formatted (e.g., by Prettier) to remove unnecessary parentheses, this pattern will fail to match.
However, the subsequent pattern `/\s*<Script[^>]*react-scan[^>]*\/>/gi` will likely succeed, removing the component but leaving the conditional wrapper `{process.env.NODE_ENV === 'development' && }`, which causes a syntax error.</violation>
</file>
<file name="packages/react-grab/src/components/toolbar/state.ts">
<violation number="1" location="packages/react-grab/src/components/toolbar/state.ts:12">
P2: Input `mode` from localStorage is not validated against the `ToolbarMode` union. If the stored JSON contains `null` (valid JSON) or an invalid string, it could be returned as-is, violating the type contract and potentially causing runtime errors in consumers.
Add validation to ensure only valid modes are returned.</violation>
</file>
<file name="packages/react-grab/src/components/icons/icon-play.tsx">
<violation number="1" location="packages/react-grab/src/components/icons/icon-play.tsx:3">
P2: This component deviates from the codebase's icon patterns (e.g., `IconCheck`, `IconChevron`):
1. Hardcoded `white` colors limit reusability (should use `currentColor`).
2. Lacks a `size` prop for scaling.
The fix aligns it with other icons and defaults to the current 6x7 size. Ensure usages (e.g., in `toolbar/index.tsx`) set a text color (like `text-white`) if relying on the previous hardcoded value.</violation>
</file>
<file name="packages/react-grab/src/components/render-scan.tsx">
<violation number="1" location="packages/react-grab/src/components/render-scan.tsx:83">
P1: `Map.groupBy` is not supported in Node.js 18 or older browsers (e.g. Chrome < 117, Safari < 17.4). Since the project supports Node 18 (`engines` or `dependencies`), this will cause runtime errors in supported environments. Use a manual grouping loop instead.</violation>
</file>
<file name="packages/react-grab/src/design-system.tsx">
<violation number="1" location="packages/react-grab/src/design-system.tsx:2577">
P2: The Play icon SVG is hardcoded here, duplicating the logic from `IconPlay`.
Use the existing `IconPlay` component (from `src/components/icons/icon-play.tsx`) to ensure consistency and reduce maintenance overhead. You will need to add `import { IconPlay } from "./components/icons/icon-play.jsx";` to the imports.</violation>
</file>
<file name="packages/cli/test/transform.test.ts">
<violation number="1" location="packages/cli/test/transform.test.ts:919">
P2: Tests do not cover valid JSX conditionals without parentheses (e.g., `&& <Script />`). The removal logic relies on finding parentheses `&& (<Script ...)` matching the CLI injection pattern, but manual installations might omit them.
Add a test case to verify robustness against this variation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,20 @@ | |||
| import type { Component } from "solid-js"; | |||
There was a problem hiding this comment.
P2: This component deviates from the codebase's icon patterns (e.g., IconCheck, IconChevron):
- Hardcoded
whitecolors limit reusability (should usecurrentColor). - Lacks a
sizeprop for scaling.
The fix aligns it with other icons and defaults to the current 6x7 size. Ensure usages (e.g., in toolbar/index.tsx) set a text color (like text-white) if relying on the previous hardcoded value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/components/icons/icon-play.tsx, line 3:
<comment>This component deviates from the codebase's icon patterns (e.g., `IconCheck`, `IconChevron`):
1. Hardcoded `white` colors limit reusability (should use `currentColor`).
2. Lacks a `size` prop for scaling.
The fix aligns it with other icons and defaults to the current 6x7 size. Ensure usages (e.g., in `toolbar/index.tsx`) set a text color (like `text-white`) if relying on the previous hardcoded value.</comment>
<file context>
@@ -0,0 +1,20 @@
+import type { Component } from "solid-js";
+
+interface IconPlayProps {
+ class?: string;
+}
</file context>
| } | ||
| > | ||
| <button class="contain-layout shrink-0 flex items-center justify-center size-3.5 rounded-full bg-black/70 hover:bg-black cursor-pointer"> | ||
| <svg |
There was a problem hiding this comment.
P2: The Play icon SVG is hardcoded here, duplicating the logic from IconPlay.
Use the existing IconPlay component (from src/components/icons/icon-play.tsx) to ensure consistency and reduce maintenance overhead. You will need to add import { IconPlay } from "./components/icons/icon-play.jsx"; to the imports.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/design-system.tsx, line 2577:
<comment>The Play icon SVG is hardcoded here, duplicating the logic from `IconPlay`.
Use the existing `IconPlay` component (from `src/components/icons/icon-play.tsx`) to ensure consistency and reduce maintenance overhead. You will need to add `import { IconPlay } from "./components/icons/icon-play.jsx";` to the imports.</comment>
<file context>
@@ -2413,10 +2552,83 @@ const StateCard = (props: StateCardProps) => {
+ }
+ >
+ <button class="contain-layout shrink-0 flex items-center justify-center size-3.5 rounded-full bg-black/70 hover:bg-black cursor-pointer">
+ <svg
+ class="ml-px"
+ width="6"
</file context>
| expect(result.success).toBe(true); | ||
| expect(result.noChanges).toBeFalsy(); | ||
| expect(result.newContent).not.toContain("react-scan"); | ||
| expect(result.newContent).toContain("<head>"); |
There was a problem hiding this comment.
P2: Tests do not cover valid JSX conditionals without parentheses (e.g., && <Script />). The removal logic relies on finding parentheses && (<Script ...) matching the CLI injection pattern, but manual installations might omit them.
Add a test case to verify robustness against this variation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/test/transform.test.ts, line 919:
<comment>Tests do not cover valid JSX conditionals without parentheses (e.g., `&& <Script />`). The removal logic relies on finding parentheses `&& (<Script ...)` matching the CLI injection pattern, but manual installations might omit them.
Add a test case to verify robustness against this variation.</comment>
<file context>
@@ -887,3 +888,340 @@ describe("applyPackageJsonTransform", () => {
+ expect(result.success).toBe(true);
+ expect(result.noChanges).toBeFalsy();
+ expect(result.newContent).not.toContain("react-scan");
+ expect(result.newContent).toContain("<head>");
+ });
+
</file context>
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| const grouped = Map.groupBy(countByComponent, ([, count]) => count); |
There was a problem hiding this comment.
Map.groupBy() is an ES2024 feature that causes runtime errors in unsupported browsers (Safari <18, Firefox <133, Chrome <130, Edge <130)
|
Bugbot Autofix prepared fixes for 3 of the 3 bugs found in the latest run.
|
|
Bugbot Autofix prepared fixes for 3 of the 3 bugs found in the latest run.
|
| stroke-linejoin="round" | ||
| > | ||
| <path d="M1 1L5 3.5L1 6V1Z" /> | ||
| </svg> |
There was a problem hiding this comment.
Duplicate play icon SVG instead of using IconPlay component
Low Severity
The play icon SVG is duplicated inline twice (lines 2577-2588 and 2603-2614) in design-system.tsx. The IconPlay component was added in this PR and is already used in toolbar/index.tsx. For consistency with IconCopy which is properly imported and used at line 2617, IconPlay should be imported and used as <IconPlay class="ml-px" /> instead of duplicating the SVG.
| stroke-linejoin="round" | ||
| > | ||
| <path d="M1 1L5 3.5L1 6V1Z" /> | ||
| </svg> |
There was a problem hiding this comment.
Two identical inline SVG play icons were duplicated in design-system.tsx instead of using the existing IconPlay component
| if (typeof navigator !== "undefined" && navigator.clipboard) { | ||
| await navigator.clipboard.writeText(text); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The copyRecording function lacks try-catch error handling around navigator.clipboard.writeText(), causing unhandled errors if clipboard operation fails
| projectInfo.packageManager, | ||
| projectInfo.projectRoot, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Package uninstall ignores detection-removal file mismatch
High Severity
Detection checks files like _app.tsx for Next.js Pages Router and src/main.tsx for Vite, but removal only targets _document.tsx or index.html respectively. When React Scan is in a detected-but-not-removed file, the package is still uninstalled unconditionally, leaving broken imports. The detectedFiles array that tracks where React Scan was found is populated but never used to prevent this inconsistency.
Additional Locations (1)
| if (typeof navigator !== "undefined" && navigator.clipboard) { | ||
| await navigator.clipboard.writeText(text); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Recording data cleared even when copy fails silently
Medium Severity
The copyRecording function silently returns without copying when navigator.clipboard is unavailable, but handleCopyRecording unconditionally calls clearLogHistory() and setHasRecordedData(false) afterward. This causes user's recorded render data to be permanently lost without being copied in environments where the clipboard API is not available (older browsers, certain security contexts).
Additional Locations (1)
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
|
| ]); | ||
| label.textWidth = context.measureText(label.text).width; | ||
| labels.delete(otherKey); | ||
| } |
There was a problem hiding this comment.
Label merge loses previously merged boxes in sequential merges
Medium Severity
When merging overlapping labels, the code updates label.text using label.boxes and otherLabel.boxes, but never updates label.boxes to include the merged result. If a third label subsequently merges into the same label, it uses the original label.boxes (not the accumulated boxes), causing previously merged render information to be lost from the displayed text.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
| logger.log("No changes needed."); | ||
| logger.break(); | ||
| process.exit(0); | ||
| } |
There was a problem hiding this comment.
Migration early exit skips package uninstall when no file changes
Medium Severity
The early exit condition !hasRemovalChanges && !hasAddChanges causes the migration to exit with "No changes needed" even when react-scan package is installed in package.json but has no file references. The later package uninstall logic at lines 389-397 correctly checks reactScanInfo.isPackageInstalled && (hasRemovalChanges || reactScanInfo.detectedFiles.length === 0), which would uninstall the package when there are no file references, but this code is never reached due to the early exit.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
|
|
||
| if (reactScanInfo.isPackageInstalled) { | ||
| logger.log(` ${pc.red("−")} Uninstall ${migrationSource} package`); | ||
| } |
There was a problem hiding this comment.
Preview shows package uninstall that won't happen
Medium Severity
The preview at line 330 shows "Uninstall react-scan package" whenever reactScanInfo.isPackageInstalled is true. However, the actual uninstall at lines 393-401 only happens when isPackageInstalled && (hasRemovalChanges || detectedFiles.length === 0). When React Scan is detected in files but can't be auto-removed (detectedFiles.length > 0 and hasRemovalChanges = false), the preview misleadingly shows the uninstall action, but it won't actually be performed.
Additional Locations (1)
| const [isEnabled, setIsEnabled] = createSignal( | ||
| savedToolbarState?.enabled ?? true, | ||
| const [toolbarMode, setToolbarMode] = createSignal<ToolbarMode>( | ||
| savedToolbarState?.mode ?? "select", |
There was a problem hiding this comment.
Saved scan mode not validated against instrumentation availability
Medium Severity
When loading the saved toolbar state, the mode value is used directly without checking if it's still valid. If the saved mode is "scan" but isInstrumentationActive() returns false on page reload (e.g., React DevTools unavailable), the toolbar state becomes inconsistent: toolbarMode is "scan" so recording controls are visible and RenderScan is rendered, but the ModeSelector shows the knob at the "off" position because it only displays 2 options when scan mode is unavailable.
packages/cli/src/commands/migrate.ts
Outdated
| } | ||
|
|
||
| exitWithMessage(); | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation makes control flow misleading
Low Severity
The if (removalResult.noChanges) block and its contents are indented at 6 spaces when they should be at 8 spaces to match their actual nesting level inside if (projectInfo.hasReactGrab). This makes the code structure visually appear as if these blocks are at the same level as the parent if, when they're actually inside it. The control flow is correct due to braces, but future maintainers may misunderstand the code structure.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
| logger.log( | ||
| ` ${pc.green("+")} Add React Grab to ${addResult.filePath}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing warning for uncleaned react-scan files during migration
Medium Severity
When React Grab is not yet installed, the migration proceeds without warning users about detected react-scan files that weren't automatically cleaned. The warning logic at lines 177-188 only triggers when projectInfo.hasReactGrab is true. In the normal migration path, getOtherDetectedFiles() may contain files with react-scan that the removal didn't target (e.g., react-scan in pages/_app.tsx when removal only targets pages/_document.tsx), but no warning is displayed.
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
|
|
||
| if (!hasRemovalChanges && !hasAddChanges) { | ||
| exitWithMessage("No changes needed."); | ||
| } |
There was a problem hiding this comment.
Migration exits silently ignoring detected React Scan files
Medium Severity
When React Scan is detected (in files or package.json) but the target transformation file doesn't exist, the migration exits with "No changes needed." without warning the user about detected React Scan files that couldn't be cleaned, and without offering to uninstall the react-scan package. This happens because previewReactScanRemoval returns noChanges: true and previewTransform returns success: false when target files are missing, causing both hasRemovalChanges and hasAddChanges to be false, which triggers the early exit before the warning logic at lines 353-363 is reached.
|
|
||
| logger.break(); | ||
| logger.success("Migration complete! React Scan has been removed."); | ||
| exitWithMessage(); |
There was a problem hiding this comment.
Missing warning about uncleaned files in hasReactGrab branch
Medium Severity
When React Grab is already installed and the removal transform succeeds, the code logs "Migration complete! React Scan has been removed." without checking for additional detected files that weren't automatically cleaned. The main migration flow (lines 353-363) includes a warning when getOtherDetectedFiles().length > 0, but this warning is missing in the hasReactGrab branch. A user could have React Scan in multiple files; only one gets cleaned, but the success message suggests the migration is fully complete.
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
|
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/cli/src/commands/migrate.ts">
<violation number="1" location="packages/cli/src/commands/migrate.ts:295">
P2: In non-interactive mode this block will uninstall react-scan even though references remain (detected files could not be cleaned), leaving broken imports/scripts. Skip uninstall when `-y` is used in this scenario or require explicit confirmation only in interactive mode.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/cli/src/commands/migrate.ts
Outdated
| uninstallPackagesWithFeedback( | ||
| ["react-scan"], | ||
| projectInfo.packageManager, | ||
| projectInfo.projectRoot, | ||
| ); | ||
| logger.break(); | ||
| logger.success("React Scan package has been uninstalled."); | ||
| } |
There was a problem hiding this comment.
P2: In non-interactive mode this block will uninstall react-scan even though references remain (detected files could not be cleaned), leaving broken imports/scripts. Skip uninstall when -y is used in this scenario or require explicit confirmation only in interactive mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/migrate.ts, line 295:
<comment>In non-interactive mode this block will uninstall react-scan even though references remain (detected files could not be cleaned), leaving broken imports/scripts. Skip uninstall when `-y` is used in this scenario or require explicit confirmation only in interactive mode.</comment>
<file context>
@@ -261,6 +274,34 @@ export const migrate = new Command()
+ "Uninstall react-scan package?",
+ isNonInteractive,
+ );
+ uninstallPackagesWithFeedback(
+ ["react-scan"],
+ projectInfo.packageManager,
</file context>
| uninstallPackagesWithFeedback( | |
| ["react-scan"], | |
| projectInfo.packageManager, | |
| projectInfo.projectRoot, | |
| ); | |
| logger.break(); | |
| logger.success("React Scan package has been uninstalled."); | |
| } | |
| if (!isNonInteractive) { | |
| await confirmOrExit( | |
| "Uninstall react-scan package?", | |
| isNonInteractive, | |
| ); | |
| uninstallPackagesWithFeedback( | |
| ["react-scan"], | |
| projectInfo.packageManager, | |
| projectInfo.projectRoot, | |
| ); | |
| logger.break(); | |
| logger.success("React Scan package has been uninstalled."); | |
| } |
| exitWithMessage(); | ||
| } | ||
| exitWithMessage("No changes needed."); | ||
| } |
There was a problem hiding this comment.
Silent failure when detected react-scan pattern cannot be removed
Medium Severity
Detection patterns in REACT_SCAN_DETECTION_PATTERNS include /<script[^>]*react-scan/i for CDN script tags, but removal patterns (removeReactScanFromVite) only handle dynamic imports like import("react-scan"). When a Vite project has a CDN script like <script src="unpkg.com/react-scan">, detection finds it, but removal fails silently with noChanges: true. The warning logic at lines 276-306 only fires when BOTH removal and addition have no changes. The warning at lines 394-404 uses getOtherDetectedFiles() which excludes the removal target file. So when removal fails but addition proceeds, the user sees "Migration complete!" with no indication that react-scan is still present.
Additional Locations (1)
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
| newContent, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Pattern removal exits after first match type
Medium Severity
The removePatternFromContent function iterates through patterns and returns immediately when the first pattern type matches, even if the file contains multiple different types of react-scan patterns. For example, if a Vite HTML file has both an inline script block containing import("react-scan") AND an external <script src="react-scan"> tag, only the first matching pattern type is removed. The file still contains react-scan after "successful" removal, and the warning at lines 394-405 in migrate.ts won't catch this because getOtherDetectedFiles() filters out removalResult.filePath.
Additional Locations (1)
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
packages/cli/src/utils/transform.ts
Outdated
| const REACT_SCAN_VOID_IMPORT = | ||
| /\s*void\s+import\s*\(\s*["']react-scan["']\s*\)\s*;?\s*/gi; | ||
| const REACT_SCAN_HTML_SCRIPT_TAG_INLINE = | ||
| /\s*<script[^>]*>[\s\S]*?react-scan[\s\S]*?<\/script>\s*/gi; |
There was a problem hiding this comment.
Overly aggressive inline script removal pattern deletes unrelated code
Medium Severity
The REACT_SCAN_HTML_SCRIPT_TAG_INLINE regex pattern /\s*<script[^>]*>[\s\S]*?react-scan[\s\S]*?<\/script>\s*/gi matches and removes entire <script> blocks containing "react-scan" anywhere in the content. When a script block contains both react-scan imports AND other code, the entire block is removed, deleting unrelated code. The test at line 1181 ("should preserve other imports in same block") expects some-other-lib to be preserved, but this pattern would remove it.
Additional Locations (1)
Co-authored-by: Cursor <cursoragent@cursor.com>
| message: noChangesMessage, | ||
| noChanges: true, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Pattern removal only applies first matching pattern
Medium Severity
The removePatternFromContent function iterates through patterns but returns immediately after the first pattern produces a change. If a file contains multiple types of react-scan references that match different patterns (e.g., both a conditional {process.env.NODE_ENV === "development" && (<Script src="react-scan" />)} AND a bare <Script src="react-scan" />), only the first matching pattern type is removed. The PR description mentions "chain all pattern replacements to ensure complete removal," but this implementation does not chain.
| [REACT_SCAN_DEV_CONDITIONAL_IMPORT, REACT_SCAN_DYNAMIC_IMPORT], | ||
| "Remove React Scan from Webpack", | ||
| "No React Scan import found", | ||
| ); |
There was a problem hiding this comment.
CDN script tags in HTML not removable by removal patterns
Medium Severity
Detection patterns catch HTML <script> tags with react-scan via /<script[^>]*react-scan/i, but removal functions for Vite and Webpack only use REACT_SCAN_DYNAMIC_IMPORT. The REACT_SCAN_SCRIPT_TAG pattern also only matches self-closing tags (\/>), not standard HTML <script src="..."></script>. This means CDN script references in index.html files will be detected but cannot be removed, leaving users with warnings about files that weren't cleaned. The PR description claims to "remove CDN HTML <script> tags... in Vite/Webpack" but the implementation doesn't support this.
- Fix removePatternFromContent to chain all pattern replacements instead of returning after first match - Add REACT_SCAN_HTML_SCRIPT_TAG pattern to match standard HTML <script></script> tags - Include HTML script tag pattern in Vite and Webpack removal functions - Run formatter on modified files
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
|
2 participants
Note
Medium Risk
Touches CLI migration logic that edits project files and installs/uninstalls packages, so mistakes in detection/regex transforms could break user configurations. Changes are mitigated by diff previews, confirmations, and broad unit test coverage.
Overview
Adds a new
grab migrateCLI command that guides users through migrating fromreact-scantoreact-grab, including framework/router preflight checks, diff previews, confirmation prompts, and install/uninstall steps.Improves detection and cleanup utilities by introducing
detectReactScan, expanding config-file scanning across common entry/layout files and extensions, and addingpreviewReactScanRemovaltransforms to stripreact-scanscripts/imports for Next.js/Vite/Webpack/TanStack. Corresponding test coverage is added/expanded for both detection and removal behavior.Updates the design-system playground to support toolbar mode/scan UI states (e.g., mode selector, recording/copy controls), and adds a
PerformanceTestcomponent in the gym dashboard to generate intentional UI jank for performance testing.Written by Cursor Bugbot for commit 13fa582. This will update automatically on new commits. Configure here.
Summary by cubic
Adds Scan mode to visualize and record React component renders, plus a migrate CLI to detect and safely remove react-scan across frameworks with guarded uninstall flows.
Bug Fixes
Migration
Written for commit 13fa582. Summary will update on new commits.