mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(react-router): read links through a route-config object's getHref (#2187)
bulletproof-react writes every link as `<Link to={paths.app.discussion.getHref(id)}>`
and every redirect as `navigate(paths.auth.login.getHref(...))`, so none of its
navigation reached a route. The href reader now follows the config object to
where it is declared (through imports and tsconfig aliases), takes the string or
template the helper returns, keeps a whole-segment `${id}` as a parameter and
drops a hole glued onto a segment (a `?redirectTo=` suffix).
The Link tag pattern also stopped at the `>` of an arrow attribute written
before `to` (`onMouseEnter={() => prefetch(id)}`), which hid those links.
bulletproof-react: 0 -> 11 navigates edges; jira_clone, ecommerce-react,
react-redux-realworld, create-t3-turbo and excalidraw unchanged.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- React Router links and `navigate(…)` calls written through a route-config object, like bulletproof-react's `paths.app.discussion.getHref(id)`, now reach the route they open. A `<Link>` with an arrow-function attribute before its `to`, such as `onMouseEnter={() => …}`, is read too.
|
||||
- C++ classes written around macros and preprocessor conditionals now keep their members. These include an access specifier spelled as a macro (`JSON_PRIVATE_UNLESS_TESTED:`), an attribute macro after a template head or a doc comment, and an `#if` in the middle of a declaration, such as inside a constructor's initializer list. The first branch of a mid-declaration `#if` is the one indexed. nlohmann/json's `basic_json` class used to be lost whole and now has its roughly 390 members. Files with a parse error dropped on redis, curl, pybind11 and leveldb, and no file got worse. Re-index after upgrading.
|
||||
- Kotlin files using newer syntax now keep their classes intact. That covers `when` guard conditions (`is X if cond ->`), `..<` ranges, multi-dollar strings (`$$"…"`) and nullable receivers in function types (`T?.() -> R`). Before, the class around such code could come out as a scatter of loose functions. On Exposed, files with a parse error dropped from 65 to 43, and on kotlinx.coroutines from 50 to 46. Re-index after upgrading.
|
||||
- A call through a class name, like `THorse.Get(…)`, `MyView.as_view()` or `ColorChoices.values()`, now reaches the class method that class inherits. The inheritance is read from the class declarations themselves, in Pascal, Python, Ruby, PHP, TypeScript/JavaScript and the Java family. Before, such a call was resolved by guessing from the name. On Horse, `THorse.Get('/ping', …)` used to land on a route-group interface's `Get` 120 times. Re-index after upgrading.
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* React Router links written through a route-config object (bulletproof-react's
|
||||
* `config/paths.ts`): `<Link to={paths.app.discussion.getHref(id)}>` and
|
||||
* `navigate(paths.auth.login.getHref())` read the href the helper returns —
|
||||
* a template's whole-segment `${id}` stays a parameter, a hole glued onto a
|
||||
* segment (`?redirectTo=…`) is dropped. An arrow-function attribute before
|
||||
* `to` (`onMouseEnter={() => …}`) no longer ends the tag early.
|
||||
*/
|
||||
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
|
||||
let root = '';
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rr-config-href-'));
|
||||
const files: Record<string, string> = {
|
||||
'package.json': JSON.stringify({ name: 'web', dependencies: { react: '^18', 'react-router': '^7' } }),
|
||||
'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@/*': ['./src/*'] } } }),
|
||||
'src/config/paths.ts': `export const paths = {
|
||||
auth: {
|
||||
login: {
|
||||
path: '/auth/login',
|
||||
getHref: (redirectTo?: string | null | undefined) =>
|
||||
\`/auth/login\${redirectTo ? \`?redirectTo=\${encodeURIComponent(redirectTo)}\` : ''}\`,
|
||||
},
|
||||
},
|
||||
app: {
|
||||
root: { path: '/app', getHref: () => '/app' },
|
||||
discussions: { path: 'discussions', getHref: () => '/app/discussions' },
|
||||
discussion: {
|
||||
path: 'discussions/:discussionId',
|
||||
getHref: (id: string) => \`/app/discussions/\${id}\`,
|
||||
},
|
||||
},
|
||||
} as const;
|
||||
`,
|
||||
'src/app/router.tsx': `import { createBrowserRouter } from 'react-router';
|
||||
import { paths } from '@/config/paths';
|
||||
|
||||
export const router = createBrowserRouter([
|
||||
{ path: paths.auth.login.path, lazy: () => import('./routes/auth/login') },
|
||||
{
|
||||
path: paths.app.root.path,
|
||||
lazy: () => import('./routes/app/root'),
|
||||
children: [
|
||||
{ path: paths.app.discussions.path, lazy: () => import('./routes/app/discussions') },
|
||||
{ path: paths.app.discussion.path, lazy: () => import('./routes/app/discussion') },
|
||||
],
|
||||
},
|
||||
]);
|
||||
`,
|
||||
'src/app/routes/auth/login.tsx': `export default function LoginRoute() { return null; }
|
||||
`,
|
||||
'src/app/routes/app/root.tsx': `export default function AppRoot() { return null; }
|
||||
`,
|
||||
'src/app/routes/app/discussions.tsx': `export default function DiscussionsRoute() { return null; }
|
||||
`,
|
||||
'src/app/routes/app/discussion.tsx': `export default function DiscussionRoute() { return null; }
|
||||
`,
|
||||
'src/features/discussions/discussions-list.tsx': `import { Link } from 'react-router';
|
||||
import { paths } from '@/config/paths';
|
||||
|
||||
export function DiscussionsList({ items, prefetch }: { items: { id: string }[]; prefetch: (id: string) => void }) {
|
||||
return items.map((d) => (
|
||||
<Link onMouseEnter={() => prefetch(d.id)} to={paths.app.discussion.getHref(d.id)}>
|
||||
open
|
||||
</Link>
|
||||
));
|
||||
}
|
||||
`,
|
||||
'src/lib/auth.tsx': `import { useNavigate } from 'react-router';
|
||||
import { paths } from '@/config/paths';
|
||||
|
||||
export function Logout() {
|
||||
const navigate = useNavigate();
|
||||
return <button onClick={() => navigate(paths.auth.login.getHref(location.pathname))}>out</button>;
|
||||
}
|
||||
`,
|
||||
};
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, rel), content);
|
||||
}
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** `route name` of every navigates edge out of a file. */
|
||||
function navigatesFrom(file: string): string[] {
|
||||
const ids = cg.getNodesInFile(file).map((n) => n.id);
|
||||
return cg
|
||||
.getOutgoingEdgesFrom(ids)
|
||||
.filter((e) => e.kind === 'navigates')
|
||||
.map((e) => cg.getNode(e.target)!.name)
|
||||
.sort();
|
||||
}
|
||||
|
||||
describe('React Router links through a route-config object', () => {
|
||||
it('reads a Link’s `to={paths.x.getHref(id)}`, past an arrow attribute before it', () => {
|
||||
expect(navigatesFrom('src/features/discussions/discussions-list.tsx')).toEqual(['/app/discussions/:discussionId']);
|
||||
});
|
||||
|
||||
it('reads `navigate(paths.x.getHref(…))`, dropping a query suffix glued onto the path', () => {
|
||||
expect(navigatesFrom('src/lib/auth.tsx')).toEqual(['/auth/login']);
|
||||
});
|
||||
});
|
||||
@@ -38,7 +38,7 @@ guessed.
|
||||
|---|---|---|---|---|
|
||||
| Expo Router | `frameworks/expo-router.ts` | `expo-router-synthesizer.ts` | `expo-router.test.ts` | — |
|
||||
| Next.js | `frameworks/nextjs.ts` | `next-router-synthesizer.ts` | `nextjs.test.ts` | next-saas-starter |
|
||||
| React Router | `frameworks/react-router.ts` | `react-router-synthesizer.ts` | `react-router.test.ts` | proshop (44 edges), proshop-v2 (28), react-redux-realworld (22), react-boilerplate (`styled(Link)`), takenote (v5 `<Redirect>`), bulletproof-react (nested `children`, `lazy` routes, `paths.x.path` constants through an app's own tsconfig alias: 0 → 9 named, linked routes); its `paths.x.getHref()` links are not read yet |
|
||||
| React Router | `frameworks/react-router.ts` | `react-router-synthesizer.ts` | `react-router.test.ts` | proshop (44 edges), proshop-v2 (28), react-redux-realworld (22), react-boilerplate (`styled(Link)`), takenote (v5 `<Redirect>`), bulletproof-react (nested `children`, `lazy` routes, `paths.x.path` constants through an app's own tsconfig alias: 0 → 9 named, linked routes), and its `paths.x.getHref(id)` links and `navigate(paths.x.getHref())` calls (0 → 11 navigates) |
|
||||
| TanStack Router | `frameworks/tanstack-router.ts` | `tanstack-router-synthesizer.ts` | `tanstack-router.test.ts` | TanStack examples, fastapi-template frontend |
|
||||
| Vue Router / Nuxt | `frameworks/vue-router.ts` (Nuxt file routes: `nuxtResolver` in `frameworks/vue.ts`) | `vue-router-synthesizer.ts` | `vue-router.test.ts` | vue-realworld (23 edges); vue-element-admin (62 routes), vue-admin-template (14), vben (192), halo console (34) — named tables, module files, `children` + layouts; Nuxt: mealie, elk, nuxt/movies |
|
||||
| SvelteKit | `frameworks/sveltekit-router.ts` | `sveltekit-synthesizer.ts` | `sveltekit-router.test.ts`, `sveltekit-route-names.test.ts` | sveltekit-realworld (31 edges); shadcn-svelte and skeleton (`(group)` layouts: 13 and 23 edges), svelte.dev (74), kit's test apps (47) |
|
||||
|
||||
@@ -54,6 +54,7 @@ import {
|
||||
// two arms). It lives in `nextjs.ts` because that is where it was first
|
||||
// needed; duplicating it here would be a second derivation of the same rule.
|
||||
import { destinationsForHref } from './nextjs';
|
||||
import { configHrefExpression } from './react';
|
||||
|
||||
const ROUTE_LANGUAGES: readonly Language[] = ['typescript', 'javascript', 'tsx', 'jsx'];
|
||||
|
||||
@@ -180,6 +181,11 @@ export const reactRouterResolver: FrameworkResolver = {
|
||||
const arg = firstArgumentText(lines, ref.line, ref.column, verb);
|
||||
if (arg === null) return null;
|
||||
let href = parseHrefExpression(arg);
|
||||
// `navigate(paths.auth.login.getHref())`: a route-config object's href.
|
||||
if (!href) {
|
||||
const configured = configHrefExpression(arg, ref.filePath, context);
|
||||
if (configured) href = parseHrefExpression(configured);
|
||||
}
|
||||
if (!href) {
|
||||
const enclosing = context.getNodeById?.(ref.fromNodeId);
|
||||
const start = enclosing && enclosing.filePath === ref.filePath ? enclosing.startLine : Math.max(1, ref.line - 40);
|
||||
|
||||
@@ -226,6 +226,112 @@ function constantPathValue(expr: string, fromFile: string, context: ResolutionCo
|
||||
return open < 0 ? null : readObjectPath(text, open, keys);
|
||||
}
|
||||
|
||||
/**
|
||||
* The href a route-config object names — `paths.app.discussion.getHref(id)`
|
||||
* or `paths.app.discussion.path` against `export const paths = { app: {
|
||||
* discussion: { path: 'discussions/:discussionId', getHref: (id: string) =>
|
||||
* \`/app/discussions/${id}\` } } }` (bulletproof-react's `config/paths.ts`)
|
||||
* — as the string or template literal it returns, ready for the href reader.
|
||||
* A `${…}` glued to a segment (a `?redirectTo=` suffix) is dropped; one
|
||||
* that is a whole segment stays a hole. Null for anything else.
|
||||
*/
|
||||
export function configHrefExpression(expr: string, fromFile: string, context: ResolutionContext): string | null {
|
||||
const m = /^\s*([A-Za-z_$][\w$]*)((?:\s*\??\.\s*[A-Za-z_$][\w$]*)+)\s*(\((?:[^()]|\([^()]*\))*\))?\s*$/.exec(expr);
|
||||
if (!m) return null;
|
||||
const root = m[1]!;
|
||||
const keys = m[2]!.split('.').map((k) => k.replace(/[?\s]/g, '')).filter(Boolean);
|
||||
let file = fromFile;
|
||||
let name = root;
|
||||
const mapping = context.getImportMappings(fromFile, 'tsx').find((x) => x.localName === root) ??
|
||||
context.getImportMappings(fromFile, 'typescript').find((x) => x.localName === root);
|
||||
if (mapping) {
|
||||
const resolved = resolveImportPath(mapping.source, fromFile, 'typescript', context);
|
||||
if (!resolved) return null;
|
||||
file = resolved;
|
||||
if (mapping.exportedName && mapping.exportedName !== 'default' && mapping.exportedName !== '*') name = mapping.exportedName;
|
||||
}
|
||||
const decl = context.getNodesInFile(file).find((n) => n.name === name && (n.kind === 'constant' || n.kind === 'variable'));
|
||||
if (!decl) return null;
|
||||
const lines = context.readFile(file)?.split('\n') ?? [];
|
||||
const text = lines.slice(decl.startLine - 1, decl.endLine).join('\n');
|
||||
const open = text.indexOf('{', Math.max(0, text.search(new RegExp(`\\b${name}\\b`))));
|
||||
if (open < 0) return null;
|
||||
let value = readObjectValue(text, open, keys);
|
||||
if (value === null) return null;
|
||||
// A function's value is what it returns: `(id: string) => \`/app/…\``, `() => { return '/'; }`.
|
||||
if (m[3] !== undefined) {
|
||||
const body = /^(?:async\s+)?(?:\([^()]*(?:\([^()]*\)[^()]*)*\)|[A-Za-z_$][\w$]*)\s*(?::\s*[^=]+?)?=>\s*/.exec(value);
|
||||
if (!body) return null;
|
||||
value = value.slice(body[0].length).trim();
|
||||
if (value.startsWith('{')) value = /\breturn\s+([`'"][\s\S]*?[`'"])\s*;?\s*}/.exec(value)?.[1] ?? '';
|
||||
}
|
||||
if (!/^[`'"]/.test(value)) return null;
|
||||
// `/auth/login${redirectTo ? … : ''}`: a hole glued to a segment is a suffix, not a segment.
|
||||
return value.startsWith('`') ? dropGluedTemplateHoles(value) : value;
|
||||
}
|
||||
|
||||
/** Remove each `${…}` of a template literal that is not a whole path segment. */
|
||||
function dropGluedTemplateHoles(template: string): string {
|
||||
let out = '';
|
||||
for (let i = 0; i < template.length; i++) {
|
||||
if (template[i] === '$' && template[i + 1] === '{') {
|
||||
let depth = 0;
|
||||
let j = i + 1;
|
||||
for (; j < template.length; j++) {
|
||||
if (template[j] === '{') depth++;
|
||||
else if (template[j] === '}' && --depth === 0) break;
|
||||
}
|
||||
const hole = template.slice(i, j + 1);
|
||||
const next = template[j + 1];
|
||||
if (out.endsWith('/') && (next === '/' || next === '`' || next === '?' || next === undefined)) out += hole;
|
||||
i = j;
|
||||
continue;
|
||||
}
|
||||
out += template[i];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Walk `keys` into the object literal opening at `at`; the final value's source text, or null. */
|
||||
function readObjectValue(text: string, at: number, keys: string[]): string | null {
|
||||
const skipString = (j: number): number => {
|
||||
const quote = text[j]!;
|
||||
for (j++; j < text.length && text[j] !== quote; j++) if (text[j] === '\\') j++;
|
||||
return j + 1;
|
||||
};
|
||||
const skipValue = (j: number): number => {
|
||||
let depth = 0;
|
||||
for (; j < text.length; j++) {
|
||||
const ch = text[j]!;
|
||||
if (ch === '"' || ch === "'" || ch === '`') { j = skipString(j) - 1; continue; }
|
||||
if (ch === '{' || ch === '[' || ch === '(') depth++;
|
||||
else if (ch === '}' || ch === ']' || ch === ')') { if (depth === 0) return j; depth--; }
|
||||
else if (ch === ',' && depth === 0) return j;
|
||||
}
|
||||
return j;
|
||||
};
|
||||
let i = at + 1;
|
||||
while (i < text.length) {
|
||||
const m = /^\s*(?:([A-Za-z_$][\w$]*)|["']([^"']+)["'])\s*:\s*/.exec(text.slice(i));
|
||||
if (!m) {
|
||||
const next = skipValue(i);
|
||||
if (text[next] !== ',') return null;
|
||||
i = next + 1;
|
||||
continue;
|
||||
}
|
||||
const key = m[1] ?? m[2]!;
|
||||
const valueAt = i + m[0].length;
|
||||
const end = skipValue(valueAt);
|
||||
if (key === keys[0]) {
|
||||
if (keys.length === 1) return text.slice(valueAt, end).trim();
|
||||
return text[valueAt] === '{' ? readObjectValue(text, valueAt, keys.slice(1)) : null;
|
||||
}
|
||||
if (text[end] !== ',') return null;
|
||||
i = end + 1;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Walk `keys` into the object literal opening at `at`; the string literal at the end, or null. */
|
||||
function readObjectPath(text: string, at: number, keys: string[]): string | null {
|
||||
const skipString = (j: number): number => {
|
||||
|
||||
@@ -38,6 +38,7 @@ import { parseHrefExpression, routesForFile, toHref, type HrefLiteral } from './
|
||||
import { matchBracket } from './frameworks/object-literal';
|
||||
import { destinationsForHref } from './frameworks/nextjs';
|
||||
import { reactRouterTable } from './frameworks/react-router';
|
||||
import { configHrefExpression } from './frameworks/react';
|
||||
import { enclosingFn, makeLineAt } from './synth-utils';
|
||||
import { resolveImportPath } from './import-resolver';
|
||||
|
||||
@@ -48,7 +49,8 @@ const LINK_TAGS = ['Link', 'NavLink', 'Navigate', 'Redirect', 'LinkContainer', '
|
||||
|
||||
/** `<Tag … to=…>` for any of `tags`, the attribute anywhere in the tag. */
|
||||
function linkTagPattern(tags: readonly string[]): RegExp {
|
||||
return new RegExp(`<(${tags.map((t) => t.replace(/[$]/g, '\\$&')).join('|')})\\b([^>]*?)\\bto\\s*=\\s*(?:"([^"]*)"|'([^']*)'|(?=\\{))`, 'g');
|
||||
// An attribute before `to` may hold an arrow (`onMouseEnter={() => …}`), whose `>` is not the tag's end.
|
||||
return new RegExp(`<(${tags.map((t) => t.replace(/[$]/g, '\\$&')).join('|')})\\b((?:[^>]|=>)*?)\\bto\\s*=\\s*(?:"([^"]*)"|'([^']*)'|(?=\\{))`, 'g');
|
||||
}
|
||||
|
||||
const LINK_TAG = linkTagPattern(LINK_TAGS);
|
||||
@@ -150,7 +152,13 @@ export async function reactRouterLinkEdges(ctx: ResolutionContext, onYield: Mayb
|
||||
const at = m.index + m[0].length;
|
||||
const close = matchBracket(safe, at);
|
||||
if (close < 0) continue;
|
||||
href = parseHrefExpression(safe.slice(at + 1, close));
|
||||
const expr = safe.slice(at + 1, close);
|
||||
href = parseHrefExpression(expr);
|
||||
// `to={paths.app.discussion.getHref(id)}`: a route-config object's href.
|
||||
if (!href) {
|
||||
const configured = configHrefExpression(expr, file, ctx);
|
||||
if (configured) href = parseHrefExpression(configured);
|
||||
}
|
||||
}
|
||||
// A relative `to` is resolved against the route this markup renders
|
||||
// under — a nesting this scan does not read, so it is not a destination.
|
||||
|
||||
Reference in New Issue
Block a user