From a2c96153dab854a13690debf8056c400afa59cdd Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Sat, 31 Oct 2026 21:48:37 -0500 Subject: [PATCH] fix(flow): a Flow library's declarations are globals of the files Flow reads `stream: ReadableStream` in react's Flow files stopped linking to `declare class ReadableStream` in `flow-typed/environments/streams.js`. PR #2736 keeps a type a module doesn't import to its own file, a declaration file or a `declare global` block, and a Flow library is none of them: a `.js` script of `declare` statements. The types a Flow library declares were lost on every file. PR #2736 never shipped. Fix: a `.js` script with a top-level `declare` statement is a Flow library, its `declare module.exports` no CommonJS export, and its top level global to the JavaScript files Flow reads, which see it as they see a `.d.ts`. A TypeScript file doesn't: its `ReadableStream` is TypeScript's own. A library's own bare type names are held to the same rule, and a top-level call `declare(...)` is no statement. Validation: tests fail on main and pass here. A/B against 4499d007: react 207 added, 3 false removed (a `Function` to an enum member, an `HTMLElement` to a property); metro 174 added (`Buffer`, `http$IncomingMessage`, Jest's types), 5 false removed; excalidraw, vscode-be, express, taxonomy, flask, gin identical. Index CPU: level (see the PR). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- __tests__/flow-library-globals.test.ts | 84 ++++++++++++++++++++++++++ src/resolution/ambient-modules.ts | 6 +- src/resolution/name-matcher.ts | 21 +++++-- 4 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 __tests__/flow-library-globals.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cb651f556..8529fbf3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -473,7 +473,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a call written through a namespace, like protocolbuffers/protobuf's `absl::StripPrefix(…)` or fmtlib/fmt's `fmt::ptr(p)`, no longer links to a class's method that only shares the function's name, like protobuf's `MultiCasePrefixStripper::StripPrefix` or a test scanner's `ptr()`: a namespace holds functions and types, not methods. A call through a namespace a macro opens now links to what that namespace declares, so fmt's `fmt::ptr(p)` reaches fmt's own `ptr` function and `fmt::day(…)` its `day` type, while a call into a library outside your project, like abseil, links to nothing. A call through a lowercase class, or an alias or typedef of one, links as before. Re-index C++ projects after upgrading. - In TypeScript and JavaScript, a call to a function's own parameter, like `isAvailable(f)` on a parameter typed `(f: string) => boolean` or `compare(a, b)` on a comparator parameter, no longer links to a function of the same name in another file, or to the file's own import of that name. Before, a parameter whose type or default value contained parentheses, as a function type does, wasn't recognized as one. A parameter or local stands in for an import of its name only where it is in scope, so a Promise executor's `resolve(…)` no longer links to an imported `path.resolve`, while a `resolve(file)` after the executor still does. Re-index TypeScript and JavaScript projects after upgrading. - In TypeScript and JavaScript, a name ending in `$`, like `syncState$`, is no longer read as a declaration of `syncState`, so a call to an imported `syncState(…)` in the same function keeps its link, and a file's own `function $()` or `const $ = …` now counts as declaring `$`. -- In TypeScript, a type a file uses without importing it, like `Omit`, `Record`, the DOM's `Event`, or a generic's own `T`, no longer links to a type of that name another module of your project declares, like a local `export type Omit` or an enum member `T`: it links to the file's own type or a global one, declared in a script `.d.ts` or a `declare global` block. A type you import, write qualified (`vscode.TextDocument`), bring in with `import X = ns.X`, or name inside a `declare module` block links as before. Re-index TypeScript projects after upgrading. +- In TypeScript, a type a file uses without importing it, like `Omit`, `Record`, the DOM's `Event`, or a generic's own `T`, no longer links to a type of that name another module of your project declares, like a local `export type Omit` or an enum member `T`: it links to the file's own type or a global one, declared in a script `.d.ts` or a `declare global` block. A type you import, write qualified (`vscode.TextDocument`), bring in with `import X = ns.X`, or name inside a `declare module` block links as before. Re-index TypeScript projects after upgrading. The declarations of a Flow library, like a `flow-typed` folder, are still global to the JavaScript files Flow reads, and to no TypeScript file. - In TypeScript React files, a type annotation no longer links to a component of the same name. When a file declares a type next to a component or value of the same name, like `type ModalRegistration` beside a `ModalRegistration` component, its annotations now link to the type while its calls still link to the component, and a type the file declares on its own, like `type Option`, or a generic's type parameter, like ``, no longer links to an `Option` or `Item` component elsewhere in the project. Before, `codegraph callers` and impact listed those annotations as uses of the component. Re-index TypeScript React projects after upgrading. - In React, a component declared through a wrapper with no render function of its own, like styled-components' `styled(ActionButton)` (with `.attrs(…)`, `.withConfig(…)` or type arguments too), emotion's and MUI's `styled(Box)(…)`, `memo(Button)` or `forwardRef(Render)`, now links to the component it wraps, so a render flow in `codegraph_explore`, a component's callers and its impact go on through the wrapper instead of stopping at it. The calls and values in a wrapper's styles, like `${(props) => theme.color(props)}` or a shared `css` block, now belong to the wrapper, as they do to any other constant, so a theme helper used only in styles has callers. A component that wraps its render function in `forwardRef(…)` or `memo(…)` now shows in the impact of the top-level constants it reads, as a function component does. A wrapper around a component that comes from a package, like react-router's `Link` or antd's `Typography.Text`, links to nothing, and the file that declares a wrapper no longer shows as referencing the component it wraps. Re-index React projects after upgrading. - In TypeScript, a type written with a qualifier, like `React.ReactNode`, `Y.Doc`, `TChat.ChatMessage` or `EQueryType.CLICKHOUSE`, now links to the member of what the qualifier names: a type from a package, like React's, no longer links to a class or component of your own that shares its name, and a member of an enum or a namespace your project declares links to that member instead of to another enum's or a same-named component. Re-index TypeScript projects after upgrading. diff --git a/__tests__/flow-library-globals.test.ts b/__tests__/flow-library-globals.test.ts new file mode 100644 index 000000000..3a78aa127 --- /dev/null +++ b/__tests__/flow-library-globals.test.ts @@ -0,0 +1,84 @@ +/** + * A Flow library definition declares globals. `flow-typed/environments/streams.js` + * is a `.js` script of `declare class ReadableStream { … }` lines, and the `[libs]` + * of a flowconfig are the same, so `stream: ReadableStream` in a file Flow checks + * names that class though no import does. #2736 kept such a name to declaration + * files and `declare global` blocks, which a `.js` file isn't, and react lost + * every type reference into its Flow libraries. + */ +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-flow-libs-')); + const files: Record = { + 'flow-typed/environments/streams.js': `declare class ReadableStream { + locked: boolean; +} +declare module 'fsx' { + declare module.exports: { read(): void }; +} +`, + 'scripts/flow/environment.js': `/* @flow */ +declare interface ConsoleTask { + run(f: () => T): T; +} +`, + 'src/other.js': 'export function other(a: any) { return a; }\n', + 'src/client.js': `// @flow +import {other} from './other'; +export function start(stream: ReadableStream, task: ConsoleTask): void { + other(stream); + other(task); +} +`, + 'ts/client.ts': `import { other } from '../src/other'; +export function typed(stream: ReadableStream): void { + other(stream); +} +`, + // No `declare` anywhere: a sample's script, global to its own program. + 'src/sample.js': 'class Sample {}\n', + 'src/uses-sample.js': `import {other} from './other'; +export function sampled(x: Sample) { return other(x); } +`, + }; + 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(); + fs.rmSync(root, { recursive: true, force: true }); +}); + +const typeRefs = (name: string): string[] => cg.getOutgoingEdges(cg.getNodesByName(name)[0]!.id) + .filter((e) => e.kind === 'references') + .map((e) => `${cg.getNode(e.target)!.filePath}:${cg.getNode(e.target)!.name}`) + .sort(); + +describe('a Flow library definition', () => { + it('declares the types a Flow file names without importing them', () => { + expect(typeRefs('start')).toEqual([ + 'flow-typed/environments/streams.js:ReadableStream', + 'scripts/flow/environment.js:ConsoleTask', + ]); + }); + + it('is no global of a TypeScript file', () => { + expect(typeRefs('typed')).toEqual([]); + }); + + it('is told from a script that declares nothing', () => { + expect(typeRefs('sampled')).toEqual([]); + }); +}); diff --git a/src/resolution/ambient-modules.ts b/src/resolution/ambient-modules.ts index a503bebb0..e4d34442d 100644 --- a/src/resolution/ambient-modules.ts +++ b/src/resolution/ambient-modules.ts @@ -244,6 +244,8 @@ export interface GlobalScopeScan { * `abstract`, `enum`, `function`, `const`, …). */ globalWords: Set; + /** A `declare` statement at the file's top level: `declare class`, never a call `declare(…)`. */ + declares: boolean; /** The file's `declare module '' { … }` blocks, whose names are the declared module's. */ moduleBlocks: readonly AmbientModuleBlock[]; /** Names an `import X = …` declaration binds: `import AutofillIpcServer = autofill.AutofillIpcServer`. */ @@ -261,6 +263,7 @@ export function scanGlobalScope(source: string): GlobalScopeScan { const globalWords = new Set(); const importAliases = new Set(); let declare = 0; // 1: `declare`, 2: `declare global` + let declares = false; let inGlobal = false; let alias = 0; // 1: `import` (or `import type`), 2: `import X` let aliasName = ''; @@ -284,13 +287,14 @@ export function scanGlobalScope(source: string): GlobalScopeScan { } if (kind === Kind.Word) { if (script) globalWords.add(`${line}:${column}`); + if (declare === 1) declares = true; declare = text === 'declare' ? 1 : declare === 1 && text === 'global' ? 2 : 0; return; } if (kind === Kind.Punct && text === '{' && declare === 2) inGlobal = true; declare = 0; }); - return { script, globalWords, moduleBlocks, importAliases }; + return { script, globalWords, declares, moduleBlocks, importAliases }; } /** One `namespace A.B { … }` statement, or one in the old spelling, `module A { … }`. */ diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 35349d5dd..a33302156 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -2009,12 +2009,22 @@ export function tsGlobalScope(file: string, context: ResolutionContext): GlobalS if (scope === undefined) { const source = context.readFile(file); scope = source === null ? null : scanGlobalScope(source); - if (scope?.script && HAS_CJS_EXPORT.test(source!)) scope = { ...scope, script: false, globalWords: new Set() }; + if (scope?.script && HAS_CJS_EXPORT.test(source!) && !isFlowLibrary(file, scope)) scope = { ...scope, script: false, globalWords: new Set() }; memo.set(file, scope); } return scope; } +/** + * Whether a file is a Flow library definition: a `.js` script with a top-level + * `declare`, which only Flow reads, like react's `flow-typed/environments` and + * the `[libs]` of its flowconfig. Its `declare module.exports` is no CommonJS + * export, and its top level is the global scope of every file Flow checks. + */ +function isFlowLibrary(file: string, scope: GlobalScopeScan): boolean { + return scope.declares && /\.[cm]?jsx?$/i.test(file); +} + /** * Whether a type written as a bare name the file doesn't import, in an ES * module, may be `n`: a declaration of the file's own, or a global one — at a @@ -2031,13 +2041,16 @@ export function tsGlobalScope(file: string, context: ResolutionContext): GlobalS * level is global only to its own program — vscode's colorize fixtures * declare `type T` and copilot's eslint fixtures `interface Node` — and a * test's or fixture's globals are its tests' only, as its `declare module` - * blocks are (see ambient-modules.ts). + * blocks are (see ambient-modules.ts). A Flow library's are for the JavaScript + * files Flow reads (isFlowLibrary): TypeScript's own `ReadableStream` is no + * `declare class` of a `flow-typed` folder beside it. */ function isTsTypeInScope(n: Node, ref: UnresolvedRef, context: ResolutionContext): boolean { if (n.filePath === ref.filePath || !TS_JS_FILE.test(n.filePath)) return true; const scope = tsGlobalScope(n.filePath, context); if (!scope) return true; - if ((scope.script && !isDeclarationFile(n.filePath)) || (isTestFile(n.filePath) && !isTestFile(ref.filePath))) return false; + const declaration = isDeclarationFile(n.filePath) || (isFlowLibrary(n.filePath, scope) && !/\.[cm]?tsx?$/i.test(ref.filePath)); + if ((scope.script && !declaration) || (isTestFile(n.filePath) && !isTestFile(ref.filePath))) return false; return scope.globalWords.has(`${n.startLine}:${n.startColumn}`); } @@ -2059,7 +2072,7 @@ function isTsBareTypeName(ref: UnresolvedRef, context: ResolutionContext): boole (typeRef && isWrittenQualified(ref, context))) return false; const scope = tsGlobalScope(ref.filePath, context); const at = { startLine: ref.line, startColumn: ref.column }; - return scope?.script === false && !scope.importAliases.has(ref.referenceName) && + return !!scope && (scope.script === false || isFlowLibrary(ref.filePath, scope)) && !scope.importAliases.has(ref.referenceName) && !scope.moduleBlocks.some((block) => inBlock(at, block)); }