From 9a96b52991c7c39cb82060da00c2f7e0dc22db33 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 27 Sep 2026 13:06:08 +0000 Subject: [PATCH] fix(react-router): bound route scans to their declarations (#1348) (#2011) * fix(react-router): bound route scans to their declarations (#1348) Fixed scan windows borrowed paths and components from neighboring or nested routes. Scan JSX opening tags and data-router objects structurally, pairing only their own attributes and properties. Add persisted-graph regressions across JavaScript and TypeScript route declarations. Co-Authored-By: Claude Opus 5.5 * fix(react-router): bound route scans to their declarations (#1348) Fixed scan windows borrowed paths and components from neighboring or nested routes. Scan JSX opening tags and data-router objects structurally, pairing only their own attributes and properties. Add persisted-graph regressions across JavaScript and TypeScript route declarations. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- __tests__/react-router.test.ts | 100 +++++++++++++- src/resolution/frameworks/react.ts | 210 +++++++++++++++++++++-------- 2 files changed, 251 insertions(+), 59 deletions(-) diff --git a/__tests__/react-router.test.ts b/__tests__/react-router.test.ts index fea8d2274..70ea8266c 100644 --- a/__tests__/react-router.test.ts +++ b/__tests__/react-router.test.ts @@ -10,7 +10,7 @@ * is what the app-root gate has to get right. Mirrors `nextjs.test.ts`. */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -495,3 +495,101 @@ describe('react-router: the shapes proshop is written in', () => { expect(screens.dropped).toBe(0); }); }); + + +describe('react-router: route declaration boundaries (#1348)', () => { + let tmpDir: string; + let cg: CodeGraph | undefined; + + afterEach(() => { + cg?.close(); + cg = undefined; + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + async function index(source: string, extension: string) { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rr-boundaries-')); + fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify({ dependencies: { react: '18' } })); + fs.writeFileSync(path.join(tmpDir, `App.${extension}`), source); + cg = CodeGraph.initSync(tmpDir); + await cg.indexAll(); + const routes = cg.getNodesByKind('route'); + return { + paths: routes.map((route) => route.name).sort(), + bindings: routes.flatMap((route) => cg!.getOutgoingEdges(route.id) + .filter((edge) => edge.kind === 'references') + .map((edge) => `${route.name}->${cg!.getNode(edge.target)?.name}`)).sort(), + }; + } + + it.each(['tsx', 'jsx', 'js'])('keeps nested/index JSX routes and long attributes local in %s', async (extension) => { + const result = await index(` + import { Routes, Route } from 'react-router-dom'; + function DashboardHome() { return null; } + function Settings() { return null; } + function Shell() { return null; } + const comparison = count + + }/> + }/> + + + } path="/sibling"/> + } + check={/}/.test('}')} + handle={{ text: 'path="/borrowed"', nested: { element: } }} + title="${'x'.repeat(600)}" path="/long"/> + }}/> + + ; + } + `, extension); + expect(result).toEqual({ + paths: ['/dashboard', '/empty', '/legacy', '/long', '/no-element', '/sibling', 'settings'], + bindings: ['/legacy->Settings', '/long->Shell', '/sibling->Settings', 'settings->Settings'], + }); + }); + + it.each(['tsx', 'jsx', 'ts', 'js'])('pairs only direct data-router properties in either order in %s', async (extension) => { + const result = await index(` + import { createBrowserRouter } from 'react-router-dom'; + function DataIndex() { return null; } + function DataSettings() { return null; } + const routes = createBrowserRouter([ + { path: '/data', children: [ + { index: true, Component: DataIndex }, + { Component: DataSettings, path: 'prefs' } + ] }, + { path: '/empty' }, + { Component: DataSettings, path: '/sibling' }, + { path: '/metadata', handle: { Component: DataIndex } }, + { Component: DataSettings, handle: { path: '/not-own' } }, + { path: '/long', handle: { text: '${'x'.repeat(600)}' }, Component: DataSettings }, + { 'Component': DataSettings, /* path: '/fake' */ 'path': '/quoted' /* trailing comment */ }, + { path: '', Component: DataSettings } + ]); + `, extension); + expect(result).toEqual({ + paths: ['/', '/long', '/quoted', '/sibling', 'prefs'], + bindings: ['/->DataSettings', '/long->DataSettings', '/quoted->DataSettings', '/sibling->DataSettings', 'prefs->DataSettings'], + }); + }); + + it('keeps nested JSX and comma-containing expressions inside their data-router property', async () => { + const result = await index(` + import { createMemoryRouter } from 'react-router-dom'; + function Shell() { return null; } + function Child() { return null; } + const router = createMemoryRouter([ + { element: hello, world, + handle: { text: "}, path: '/fake'", callback: () => ({ path: '/also-fake' }) }, path: '/shell' }, + { path: '/none', handle: { element: } }, + { path: '/child', element: } + ]); + `, 'tsx'); + expect(result).toEqual({ paths: ['/child', '/shell'], bindings: ['/child->Child', '/shell->Shell'] }); + }); +}); diff --git a/src/resolution/frameworks/react.ts b/src/resolution/frameworks/react.ts index 28776cddb..863aa764e 100644 --- a/src/resolution/frameworks/react.ts +++ b/src/resolution/frameworks/react.ts @@ -98,21 +98,10 @@ export const reactResolver: FrameworkResolver = { // which is why 'tsx'/'jsx' are now in `languages` — ``/`element={}` // routes live in JSX files and were previously skipped entirely. - // React Router: (v5) or - // }/> (v6). Attributes appear in any order, - // and element={...} contains a nested `>`, so scan a window after each - // }` / `Component: Comp` (a forward window confirms - // it's a route object, not a stray `path:` field). - if (/\b(?:createBrowserRouter|createHashRouter|createMemoryRouter|createRoutesFromElements)\b/.test(content)) { - const objPathRe = /\bpath\s*:\s*['"]([^'"]*)['"]/g; - let om: RegExpExecArray | null; - while ((om = objPathRe.exec(content)) !== null) { - const win = content.slice(om.index, om.index + 300); - const compMatch = - win.match(/\belement\s*:\s*<\s*([A-Z][A-Za-z0-9_]*)/) || - win.match(/\bComponent\s*:\s*([A-Z][A-Za-z0-9_]*)/); - if (!compMatch) continue; // require a component → it's a real route object - const routePath = om[1] || '/'; - const line = content.slice(0, om.index).split('\n').length; - const routeNode: Node = { - id: `route:${filePath}:${line}:${routePath}`, - kind: 'route', - name: routePath, - qualifiedName: `${filePath}::route:${routePath}`, - filePath, - startLine: line, - endLine: line, - startColumn: 0, - endColumn: 0, - language: filePath.endsWith('.tsx') ? 'tsx' : 'jsx', - updatedAt: now, - }; - nodes.push(routeNode); - references.push({ - fromNodeId: routeNode.id, - referenceName: compMatch[1]!, + referenceName: component, referenceKind: 'references', line, column: 0, @@ -187,6 +135,152 @@ export const reactResolver: FrameworkResolver = { }, }; +interface RouteDeclaration { + path: string; + component?: string; + at: number; +} + +/** Structural scanner: strings, comments, JSX and balanced expressions are units. */ +function scanRouteDeclarations(source: string, allowJsx: boolean): RouteDeclaration[] { + const routes: RouteDeclaration[] = []; + const dataRouter = /\b(?:createBrowserRouter|createHashRouter|createMemoryRouter|createRoutesFromElements)\b/.test(source); + if (!dataRouter && !/ { + const match = /^(?:"((?:\\.|[^"\\])*)"|'((?:\\.|[^'\\])*)')$/.exec(value.trim()); + return match ? match[1] ?? match[2] : undefined; + }; + const componentName = (value: string | undefined, jsx: boolean): string | undefined => { + if (!value) return undefined; + const match = jsx + ? /^\s*<\s*([A-Z][\w]*)\s*(?=[\s/>])/.exec(value) + : /^\s*([A-Z][\w]*)\s*$/.exec(value); + return match?.[1]; + }; + // The few characters before `at`, trailing whitespace skipped: enough for the + // end-anchored checks below without copying the whole prefix per `/` or `<`. + const tokenBefore = (at: number): string => { + let j = at - 1; + while (j >= 0 && /\s/.test(source[j]!)) j--; + return source.slice(Math.max(0, j - 11), j + 1); + }; + const trivia = (at: number): number => { + while (at < source.length) { + if (/\s/.test(source[at]!)) { at++; continue; } + if (source.startsWith('//', at)) { + const end = source.indexOf('\n', at + 2); + at = end < 0 ? source.length : end; + } else if (source.startsWith('/*', at)) { + const end = source.indexOf('*/', at + 2); + at = end < 0 ? source.length : end + 2; + } else break; + } + return at; + }; + function unit(at: number): number { + const ch = source[at]; + if (ch === '"' || ch === "'" || ch === '`') { + let i = at + 1; + while (i < source.length) { + if (source[i] === '\\') { i += 2; continue; } + if (source[i] === ch) return i + 1; + if (ch === '`' && source.startsWith('${', i)) { i = unit(i + 1); continue; } + i++; + } + return i; + } + // A regex can contain braces or JSX-looking text without ending an expression. + if (ch === '/') { + const before = tokenBefore(at); + if (!before || /[=(:,[!&|?{};]$/.test(before) || /\b(?:return|throw|case|yield)\s*$/.test(before)) { + let inClass = false; + for (let i = at + 1; i < source.length && source[i] !== '\n'; i++) { + if (source[i] === '\\') { i++; continue; } + if (source[i] === '[') inClass = true; + else if (source[i] === ']') inClass = false; + else if (source[i] === '/' && !inClass) { + i++; + while (/[a-z]/i.test(source[i] ?? '') && i < source.length) i++; + return i; + } + } + } + } + if (allowJsx && ch === '<' && /^<(?:[A-Za-z][\w.:-]*|>)/.test(source.slice(at))) { + const before = tokenBefore(at); + // `count()` are not JSX opening tags. + if (!/[\w$)\]'"]$/.test(before) || /\breturn$/.test(before)) return jsx(at); + } + const close = ch === '{' ? '}' : ch === '[' ? ']' : ch === '(' ? ')' : undefined; + if (!close) return at + 1; + let i = at + 1; + const fields = new Map(); + while ((i = trivia(i)) < source.length && source[i] !== close) { + // A property must start at an object entry, never inside its value. + const key = ch === '{' ? /^(?:([A-Za-z_$][\w$]*)|["']([^"']+)["'])\s*:/.exec(source.slice(i)) : null; + if (key) { + const start = i; + const valueAt = trivia(i + key[0].length); + i = valueAt; + let valueEnd = i; + while ((i = trivia(i)) < source.length && source[i] !== ',' && source[i] !== close) { + i = unit(i); + valueEnd = i; + } + fields.set(key[1] ?? key[2]!, { value: source.slice(valueAt, valueEnd).trim(), at: start }); + } else { + // Skip a whole entry (spread, method, shorthand), but still visit nested units. + while ((i = trivia(i)) < source.length && source[i] !== ',' && source[i] !== close) i = unit(i); + } + if (source[i] === ',') i++; + } + if (dataRouter && ch === '{' && source[i] === close) { + const pathField = fields.get('path'); + const path = pathField && literal(pathField.value); + const component = componentName(fields.get('element')?.value, true) + ?? componentName(fields.get('Component')?.value, false); + if (path !== undefined && component) routes.push({ path: path || '/', component, at: pathField!.at }); + } + return i < source.length ? i + 1 : i; + } + function jsx(at: number): number { + const tag = /^<([\w.:-]*)/.exec(source.slice(at))!; + let i = at + tag[0].length; + const attrs = new Map(); + while ((i = trivia(i)) < source.length && source[i] !== '>' && !source.startsWith('/>', i)) { + const attr = /^[\w:-]+/.exec(source.slice(i)); + if (!attr) { i = unit(i); continue; } + i = trivia(i + attr[0].length); + if (source[i] !== '=') continue; + i = trivia(i + 1); + const start = i; + i = unit(i); + attrs.set(attr[0], source.slice(start, i)); + } + if (tag[1] === 'Route' && i < source.length) { + const path = literal(attrs.get('path') ?? ''); + const expression = (name: string) => attrs.get(name)?.replace(/^\{([\s\S]*)\}$/, '$1'); + const component = componentName(expression('component'), false) ?? componentName(expression('element'), true); + if (path) routes.push({ path, component, at }); + } + if (source.startsWith('/>', i)) return i + 2; + i++; + while (i < source.length) { + if (source.startsWith('', i + 2); + return end < 0 ? source.length : end + 1; + } + if (source[i] === '<' && /^<(?:[A-Za-z]|>)/.test(source.slice(i))) i = jsx(i); + else if (source[i] === '{') i = unit(i); + else i++; + } + return i; + } + let at = 0; + while ((at = trivia(at)) < source.length) at = unit(at); + return routes.sort((a, b) => a.at - b.at); +} + /** * Check if string is PascalCase */