mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 16:52:09 +08:00
fix(js): an export default line inside a template literal no longer names the default (#3068)
statedDefaultExport reads a JS/TS file's own export default statement
by scanning comment-stripped source for line-initial `export default`
heads. Strings are deliberately not masked there (a masker that loses
step on nested templates blanked real statements), and a head counted
when it named a node of the file, the first such head winning. A code
generator's template text breaks that: in
export function App() {} export function Main() {}
export const template = `
export default App
`;
export default Main;
the template's line names the real App, so `import X from './gen'`
bound to App instead of Main (issue #2968). A template holding
`export default () => …` also made a module whose real default is an
expression read as anonymous, which keeps every guess away.
templateLiteralTextAt (strip-comments.ts) scans the source once for
template-literal text: it follows `${…}` nesting (a template inside an
interpolation, a brace inside a regex there), and skips comments,
quoted strings and regex literals, which may hold a backtick; a `/`
after a value divides, anywhere else it opens a regex. A head inside
template text is now skipped. When some template never closes the scan
vouches for nothing and every head counts, as before. Nothing else
changes: a head outside template text is read exactly as it was.
Validation: default-export-declaration.test.ts gains the issue's file,
a template returned by a function behind a nested template, one passed
at module scope and one in a class field, a template's anonymous
default above an expression default, and controls (a regex and a string
holding a backtick, a division after a parenthesis, and a file whose
scan can't close a template); strip-comments.test.ts tests the scan
directly. Red with main's resolver: 2 of 10 failed; green: all pass.
Checked against tree-sitter on every line-initial head of 4,191
JS/TS files in 16 corpora (outline, excalidraw, vben, bitwarden apps,
mastodon, shadcn docs, zod, bulletproof-react, express, astro, react,
trezor-suite, this repo's src, tests and ui): of 4,149 real export
default statements the scan puts none inside template text, and all
322 heads it does place there are template text. A/B on excalidraw
d3a9c2a1, vben 61c07936, astro e3bec3d2 (packages/astro/src and
packages/integrations), with gin-gonic/gin 0f09c3a9 and psf/requests
3c7baaa0 as controls, both arms on main 4b0f72b7 through the native
kernel (and again on 66449255 through wasm): all six byte-identical,
since no default import there is decided by template text; index time
unchanged.
Not in this change: hasEsmDefaultExport, which only gates
require('./x').default, still reads the raw text.
Reimplements PR #3042 by @johnhldiver-beep.
Fixes #2968
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
617769279d
commit
ed6b88e53a
@@ -351,6 +351,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- `codegraph sync` and the file watcher now move a React Router route to its page when the router file loads that page lazily into a value of its own, like `const Docs = lazy(() => import('./pages/Docs'))` rendered by `<Route path="/docs" element={<Docs />} />`, and the page's file is added after the router was indexed, comes back after being deleted, or gets or changes its default export in a later edit. Before, the route kept pointing at the `Docs` declaration, or at the page's old component, until the router file itself changed or the project was re-indexed, unless the page's component happened to share the declaration's name. The same now holds for a `lazy: () => import('./pages/Docs')` route whose page switches its default export to another component. Re-index React projects after upgrading to correct routes an earlier version left behind.
|
||||
- `codegraph sync` and the file watcher now link a component to the JSX that renders it, like `<Team />`, when the component is added in a file with no JSX of its own, as in `export default function Team() { return null; }`. Before, the link waited for an unrelated change to a file with JSX, or for a re-index, so the new component looked unused.
|
||||
- In JavaScript and TypeScript, a default import now links to what the module's `export default` statement exports, wherever the statement sits in the file. Before, a module that declares its default, like `export default function Vans() {…}` or `export default class Store {…}`, had its default import linked to the first function it exports instead: in a React Router data-router app, where a page exports its `loader` or `action` above the page component, `import Vans, { loader as vansLoader } from './pages/Vans'` made the route `/vans` and every use of `Vans` lead to the loader. A component file that also exports a styled or memoized component no longer hands its default import to that component, and an anonymous default, like `export default function () {…}` or `export default () => …`, is no longer linked to a function exported beside it or nested inside it. A module loaded with `require('./x').default` is read the same way. Re-index JavaScript and TypeScript projects after upgrading.
|
||||
- In JavaScript and TypeScript, a line like `export default App` written inside a template literal, as code generators and build plugins write the modules they produce, no longer counts as the module's own default export. Before, when that text named a function the file declares, a default import of the module linked to that function instead of the one the real `export default` statement names, and an unnamed `export default () => …` in such text could leave the default import linked to nothing. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and @johnhldiver-beep for the first fix. (#2968)
|
||||
- In React and Vue, a tag for a component the file imports from a package, like `<Button>` from antd, `<Typography>` from `@mui/material/Typography`, `<Link>` from react-router-dom or `<Card>` from ant-design-vue, no longer links to a component, function or class in your repository that only shares its name. Before, the tag linked to whatever the repository had by that name, so `codegraph_explore`, callers and impact followed renders that never happen: in SigNoz, antd's `<Button>` tags showed up as callers of a styled link on its 404 page, and in Material UI dashboards like mantis and berry, `<Typography>` and `<Button>` tags showed up as callers of the theme's style overrides. A package here is one that a `package.json` above the file lists and that no file in the repository answers to. A tag imported through a path alias like `components/Modal`, from a workspace package of the repository, or under a name no `package.json` lists still links as before. Re-index React and Vue projects after upgrading.
|
||||
- In React with TypeScript, a component that renders a generic component and passes it type arguments, like outline's `<PaginatedList<Document> items={…} />` or excalidraw's `<DropdownMenuItemContentRadio<"contain" | "overlap"> …>`, now links to the component it renders. Before, a tag followed by type arguments was skipped, so the generic component looked unused by the screens that render it, and `codegraph_explore`'s call paths, callers and impact stopped short of it. Re-index React projects after upgrading.
|
||||
- In React, a component that a file declares itself as a value is now what that file's JSX renders, whatever else in the repository shares its name. A lazily loaded page, like `const RegisterPage = Loadable(lazy(() => import('./pages/Register')))`, a `dynamic(() => import(…))` component or one loaded only in development (`IS_DEV ? lazy(…) : null`), links to the component its module exports, including the one its loader picks and one an `export default observer(Login)` wraps; a wrapped component like `const Settings = observer(function Settings() {…})`, `React.memo(Rows, areEqual)` or `forwardRef(ComboboxInner) as {…}` links to the function it wraps; and an alias like `const DialogPortal = DialogPrimitive.Portal` links to its own declaration. Before, `<RegisterPage />` was linked to any lone component of that name elsewhere, often in another app of the same repository, so `codegraph_explore`, callers and impact followed it into the wrong app while the page it really renders had no caller. React Router layouts declared this way follow the same rule, and a TSX type named like a value the file declares now stays in that file. Re-index React projects after upgrading.
|
||||
|
||||
@@ -205,6 +205,90 @@ export default class AnimatedColor {
|
||||
const { default: Color } = require("./AnimatedColor")
|
||||
return [new AnimatedColor("red"), new Color("blue")]
|
||||
}
|
||||
`,
|
||||
// An `export default` line written in a template literal is text, not the
|
||||
// module's statement (#2968). A code generator's template names the file's
|
||||
// own `App`, above the real default — the issue's own example first.
|
||||
'src/gen/issue.ts': `export function App() { return 1; }
|
||||
export function Main() { return 2; }
|
||||
export const template = \`
|
||||
export default App
|
||||
\`;
|
||||
export default Main;
|
||||
`,
|
||||
// Returned by a function, behind a template nested in an interpolation.
|
||||
'src/gen/render.ts': `export function App() { return 1 }
|
||||
export function Main() { return 2 }
|
||||
export function render() {
|
||||
return \`intro \${\`nested\`}
|
||||
export default App
|
||||
\`
|
||||
}
|
||||
export default Main
|
||||
`,
|
||||
// Passed at module scope, where no declaration encloses it, and a class field.
|
||||
'src/gen/write.ts': `import { writeFileSync } from "fs"
|
||||
export function App() { return 1 }
|
||||
export function Main() { return 2 }
|
||||
writeFileSync("out.ts", \`
|
||||
export default App
|
||||
\`)
|
||||
export class Emitter {
|
||||
sample = \`
|
||||
export default App
|
||||
\`
|
||||
}
|
||||
export default Main
|
||||
`,
|
||||
// A template's anonymous default made the expression default below it
|
||||
// anonymous too, which keeps every guess away.
|
||||
'src/gen/model.ts': `export class Model {
|
||||
save() {
|
||||
return 1
|
||||
}
|
||||
}
|
||||
export const usage = \`
|
||||
export default class extends Model {}
|
||||
\`
|
||||
export default observable(Model)
|
||||
function observable<T>(target: T): T {
|
||||
return target
|
||||
}
|
||||
`,
|
||||
// No template text: a regex and a string that hold a backtick, and a
|
||||
// division after a parenthesis.
|
||||
'src/gen/plain.ts': `export function App() { return 1 }
|
||||
export function Main() { return 2 }
|
||||
const tick = /[\`]/g
|
||||
const half = (App() + Main()) / 2 + "\`"
|
||||
export default Main
|
||||
`,
|
||||
// A regex right after `if (…)` reads as a division, so its backtick opens a
|
||||
// literal the scan never closes: then the scan vouches for nothing, and the
|
||||
// statement still counts.
|
||||
'src/gen/unbalanced.ts': `export function App() { return 1 }
|
||||
export function Main() { return 2 }
|
||||
export function check(s: string, ok: boolean) {
|
||||
if (ok) /\`/.test(s)
|
||||
return s
|
||||
}
|
||||
export default Main
|
||||
`,
|
||||
'src/gen/use.ts': `import Issue from "./issue"
|
||||
import Render from "./render"
|
||||
import Write from "./write"
|
||||
import Persisted from "./model"
|
||||
import Plain from "./plain"
|
||||
import Unbalanced from "./unbalanced"
|
||||
|
||||
export function generate() {
|
||||
Issue()
|
||||
Render()
|
||||
Write()
|
||||
new Persisted()
|
||||
Plain()
|
||||
Unbalanced()
|
||||
}
|
||||
`,
|
||||
'src/use.tsx': `import handler from "./api/vans"
|
||||
import Store from "./store"
|
||||
@@ -333,3 +417,36 @@ describe('a default import of a module that exports something above its default'
|
||||
expect(consumed()).toContain('src/service.ts:run');
|
||||
});
|
||||
});
|
||||
|
||||
describe('an `export default` line inside a template literal', () => {
|
||||
/** `file:name` of every node `generate` calls or instantiates. */
|
||||
function generated(): string[] {
|
||||
const generate = cg.getNodesInFile('src/gen/use.ts').find((n) => n.name === 'generate')!;
|
||||
return cg
|
||||
.getOutgoingEdges(generate.id)
|
||||
.filter((e) => e.kind === 'calls' || e.kind === 'instantiates')
|
||||
.map((e) => cg.getNode(e.target)!)
|
||||
.map((n) => `${n.filePath}:${n.name}`)
|
||||
.sort();
|
||||
}
|
||||
|
||||
it('is text, so the real statement below it names the default', () => {
|
||||
const targets = generated();
|
||||
for (const file of ['issue', 'render', 'write']) {
|
||||
expect(targets).toContain(`src/gen/${file}.ts:Main`);
|
||||
expect(targets).not.toContain(`src/gen/${file}.ts:App`);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not make an expression default anonymous', () => {
|
||||
expect(generated()).toContain('src/gen/model.ts:Model');
|
||||
});
|
||||
|
||||
it('leaves regexes, strings and division alone, and a scan it cannot finish', () => {
|
||||
const targets = generated();
|
||||
expect(targets).toContain('src/gen/plain.ts:Main');
|
||||
expect(targets).toContain('src/gen/unbalanced.ts:Main');
|
||||
expect(targets).not.toContain('src/gen/plain.ts:App');
|
||||
expect(targets).not.toContain('src/gen/unbalanced.ts:App');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { stripCommentsForRegex } from '../src/resolution/strip-comments';
|
||||
import { stripCommentsForRegex, templateLiteralTextAt } from '../src/resolution/strip-comments';
|
||||
|
||||
describe('stripCommentsForRegex', () => {
|
||||
it('python: strips line comments', () => {
|
||||
@@ -132,3 +132,38 @@ real = 1
|
||||
expect(out.length).toBe(src.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('templateLiteralTextAt', () => {
|
||||
/** Whether the first `marker` in `src` is template text; null when the scan gives up. */
|
||||
function inText(src: string, marker: string): boolean | null {
|
||||
const at = templateLiteralTextAt(src);
|
||||
return at === null ? null : at(src.indexOf(marker));
|
||||
}
|
||||
|
||||
it('tells a template text from the code around it and in its interpolations', () => {
|
||||
const src = 'const a = `TEXT ${CODE} MORE`;\nAFTER';
|
||||
expect(inText(src, 'TEXT')).toBe(true);
|
||||
expect(inText(src, 'CODE')).toBe(false);
|
||||
expect(inText(src, 'MORE')).toBe(true);
|
||||
expect(inText(src, 'AFTER')).toBe(false);
|
||||
});
|
||||
|
||||
it('follows a template nested in an interpolation', () => {
|
||||
expect(inText('const a = `x ${`inner`} y\nTAIL\n`;', 'TAIL')).toBe(true);
|
||||
});
|
||||
|
||||
it('skips a regex that holds a backtick or a brace', () => {
|
||||
expect(inText('const re = /[`]/g;\nAFTER', 'AFTER')).toBe(false);
|
||||
expect(inText('const s = `h-${t.replace(/[}`]/g, "")} TAIL`;\nAFTER', 'TAIL')).toBe(true);
|
||||
expect(inText('const s = `h-${t.replace(/[}`]/g, "")} TAIL`;\nAFTER', 'AFTER')).toBe(false);
|
||||
});
|
||||
|
||||
it('skips comments and strings that hold a backtick, and divides after a value', () => {
|
||||
const src = '// a ` b\nconst s = "`" + \'`\';\n/* ` */\nconst h = (a + b) / 2 + "`";\nAFTER';
|
||||
expect(inText(src, 'AFTER')).toBe(false);
|
||||
});
|
||||
|
||||
it('gives up when a template never closes', () => {
|
||||
expect(inText('if (ok) /`/.test(s)\nAFTER', 'AFTER')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -12,7 +12,7 @@ import { UnresolvedRef, ResolvedRef, ResolutionContext, ImportMapping, ReExport
|
||||
import { applyAliases, aliasAndBaseUrlCandidates } from './path-aliases';
|
||||
import { extractLocalExportAliases } from './alias-binding';
|
||||
import { resolveWorkspaceImport, workspaceImportTargets, type WorkspaceImportTargets } from './workspace-packages';
|
||||
import { stripCommentsForRegex } from './strip-comments';
|
||||
import { stripCommentsForRegex, templateLiteralTextAt } from './strip-comments';
|
||||
import { makeLineAt } from './synth-utils';
|
||||
import { dartDirectiveFile } from './dart-libraries';
|
||||
import { ambientModuleDeclares, findAmbientModuleExport, findAmbientModuleFile, inBlock, isInModuleBlock, namespaceBlocks, type NamespaceBlock } from './ambient-modules';
|
||||
@@ -207,8 +207,12 @@ function readDefaultExport(code: string, at: number): { name: string; nameAt: nu
|
||||
* template whose `${…}` holds another template, or a regex with a backtick,
|
||||
* sets a masker out of step, and it blanked the real statement after it
|
||||
* (outline's 2,900-line styled-components `Styles.ts`). A statement must
|
||||
* name a node of the file instead — one a template only writes as text
|
||||
* names none — and of several statements, one that does wins.
|
||||
* name a node of the file instead, and of several statements, one that does
|
||||
* wins. A line of a template's text is no statement either, though it can
|
||||
* name one: a code generator's `export default App` names the file's own
|
||||
* `App` (#2968). So a head inside template text is skipped, when a scan that
|
||||
* follows `${…}` nesting and regex literals closes every template; one that
|
||||
* doesn't vouches for nothing, and every head counts.
|
||||
*/
|
||||
function statedDefaultExport(filePath: string, idx: FileExportIndex, context: ResolutionContext): StatedDefault {
|
||||
if (idx.statedDefault !== undefined) return idx.statedDefault;
|
||||
@@ -217,8 +221,10 @@ function statedDefaultExport(filePath: string, idx: FileExportIndex, context: Re
|
||||
if (source && /\bexport\s+default\b/.test(source)) {
|
||||
const code = stripCommentsForRegex(source, 'typescript');
|
||||
const lineAt = makeLineAt(code, 1);
|
||||
const inTemplateText = templateLiteralTextAt(source);
|
||||
EXPORT_DEFAULT_HEAD.lastIndex = 0;
|
||||
for (let m = EXPORT_DEFAULT_HEAD.exec(code); m !== null; m = EXPORT_DEFAULT_HEAD.exec(code)) {
|
||||
if (inTemplateText?.(m.index)) continue;
|
||||
const value = readDefaultExport(code, m.index + m[0].length);
|
||||
if (value === 'anonymous') {
|
||||
stated ??= 'anonymous';
|
||||
|
||||
@@ -82,6 +82,93 @@ export function blankStringContents(text: string, regexLiterals?: number[]): str
|
||||
return out.join('');
|
||||
}
|
||||
|
||||
/** A keyword after which a `/` opens a regex literal, at the end of the text before it. */
|
||||
const REGEX_AFTER_KEYWORD = /(?:^|[^\w$])(?:return|throw|case|yield|await|else|do|typeof|void|delete|new|in|of|instanceof)$/;
|
||||
|
||||
/**
|
||||
* Whether the `/` at `at` opens a regex literal: it does where an expression
|
||||
* starts, and divides after a value — a `)`, `]`, string, name or number.
|
||||
*/
|
||||
function slashOpensRegex(src: string, at: number): boolean {
|
||||
let j = at - 1;
|
||||
while (j >= 0 && (src[j] === ' ' || src[j] === '\t' || src[j] === '\n' || src[j] === '\r')) j--;
|
||||
if (j < 0) return true;
|
||||
const before = src[j]!;
|
||||
if (before === ')' || before === ']' || before === '"' || before === "'" || before === '`') return false;
|
||||
if (/[\w$]/.test(before)) return REGEX_AFTER_KEYWORD.test(src.slice(Math.max(0, j - 10), j + 1));
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Where JS/TS template-literal text lies in `src`: a test of whether offset
|
||||
* `at` falls in a template's literal text — not its `${…}` code — or null when
|
||||
* a template never closes, so the scan vouches for nothing. Offsets match
|
||||
* {@link stripCommentsForRegex}'s output, which keeps them.
|
||||
*
|
||||
* Unlike {@link blankStringContents} it follows `${…}` nesting (a template in
|
||||
* an interpolation, a brace in a regex there) and skips comments, strings and
|
||||
* regex literals, which may hold a backtick (`/[`]/`).
|
||||
*/
|
||||
export function templateLiteralTextAt(src: string): ((at: number) => boolean) | null {
|
||||
/** Template text spans, `[start, end)` pairs in order. */
|
||||
const spans: number[] = [];
|
||||
/** Innermost last: an open template's current text start, or an open `${…}`'s unclosed braces. */
|
||||
const open: Array<{ text: number } | { braces: number }> = [];
|
||||
const n = src.length;
|
||||
for (let i = 0; i < n; i++) {
|
||||
const c = src[i];
|
||||
const top = open[open.length - 1];
|
||||
if (top && 'text' in top) {
|
||||
if (c === '\\') i++;
|
||||
else if (c === '`') {
|
||||
spans.push(top.text, i);
|
||||
open.pop();
|
||||
} else if (c === '$' && src[i + 1] === '{') {
|
||||
spans.push(top.text, i);
|
||||
open.push({ braces: 0 });
|
||||
i++;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (c === '`') open.push({ text: i + 1 });
|
||||
else if (c === '"' || c === "'") {
|
||||
for (i++; i < n && src[i] !== c && src[i] !== '\n'; i++) if (src[i] === '\\') i++;
|
||||
} else if (c === '/' && src[i + 1] === '/') {
|
||||
while (i < n && src[i] !== '\n') i++;
|
||||
} else if (c === '/' && src[i + 1] === '*') {
|
||||
const close = src.indexOf('*/', i + 2);
|
||||
i = close < 0 ? n : close + 1;
|
||||
} else if (c === '/' && slashOpensRegex(src, i)) {
|
||||
let inClass = false;
|
||||
for (let j = i + 1; j < n && src[j] !== '\n'; j++) {
|
||||
if (src[j] === '\\') j++;
|
||||
else if (src[j] === '[') inClass = true;
|
||||
else if (src[j] === ']') inClass = false;
|
||||
else if (src[j] === '/' && !inClass) {
|
||||
i = j;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} else if (top && c === '{') top.braces++;
|
||||
else if (top && c === '}' && top.braces-- === 0) {
|
||||
open.pop();
|
||||
(open[open.length - 1] as { text: number }).text = i + 1;
|
||||
}
|
||||
}
|
||||
if (open.length > 0) return null;
|
||||
return (at) => {
|
||||
// The first span ending after `at`, then whether it starts at or before it.
|
||||
let lo = 0;
|
||||
let hi = spans.length / 2;
|
||||
while (lo < hi) {
|
||||
const mid = (lo + hi) >> 1;
|
||||
if (spans[2 * mid + 1]! <= at) lo = mid + 1;
|
||||
else hi = mid;
|
||||
}
|
||||
return lo < spans.length / 2 && spans[2 * lo]! <= at;
|
||||
};
|
||||
}
|
||||
|
||||
export type CommentLang =
|
||||
| 'python'
|
||||
| 'javascript'
|
||||
|
||||
Reference in New Issue
Block a user