fix(javascript): ignore erased TypeScript names in native-addon requests (#2766)
* fix(javascript): ignore erased TypeScript names in native-addon requests Skip import type / export type declarations and per-binding type specifiers when collecting .node addon members so only runtime value imports request native exports (fixes #2667). Co-authored-by: Cursor <cursoragent@cursor.com> * test(javascript): cover erased TypeScript re-export forms * test(javascript): allow Inspector observation events to arrive * test(inspector): emit events before enable responses --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
parent
bee73fcb67
commit
06739aeaf2
|
|
@ -0,0 +1,98 @@
|
|||
import { mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { afterEach, expect, it } from "vitest";
|
||||
|
||||
import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js";
|
||||
import type { JavaScriptApplicationAnalysisResult } from "../../domain/javascript/javascriptApplicationAnalysis.js";
|
||||
import { buildJavaScriptNativeHandoffs } from "../../domain/javascript/javascriptNativeHandoff.js";
|
||||
import { analyzeJavaScriptApplication } from "./JavaScriptApplicationService.js";
|
||||
|
||||
const fixtures: string[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(
|
||||
fixtures
|
||||
.splice(0)
|
||||
.map((path) => rm(path, { recursive: true, force: true })),
|
||||
);
|
||||
});
|
||||
|
||||
const analyzeFixture = async (source: string) => {
|
||||
const fixture = await mkdtemp(join(tmpdir(), "rea-native-types-"));
|
||||
fixtures.push(fixture);
|
||||
const scope = new ArtifactResourceScope();
|
||||
try {
|
||||
await writeFile(join(fixture, "main.ts"), `${source}\n`);
|
||||
await writeFile(join(fixture, "addon.node"), "");
|
||||
const result = await analyzeJavaScriptApplication(
|
||||
{ input_path: fixture, format: "directory" },
|
||||
scope,
|
||||
);
|
||||
if (!result.ok) throw result.error;
|
||||
return result.value
|
||||
.normalized_result as unknown as JavaScriptApplicationAnalysisResult;
|
||||
} finally {
|
||||
await scope.close();
|
||||
}
|
||||
};
|
||||
|
||||
it("does not request native-addon members from erased TypeScript names", async () => {
|
||||
const typeImport = await analyzeFixture(
|
||||
'import type { NativeThing } from "./addon.node";',
|
||||
);
|
||||
expect(typeImport.summary.native_addon_bindings).toBe(0);
|
||||
expect(typeImport.summary.resolved_native_addon_bindings).toBe(0);
|
||||
expect(
|
||||
typeImport.graph.nodes.filter((node) => node.kind === "native-export"),
|
||||
).toEqual([]);
|
||||
expect(
|
||||
buildJavaScriptNativeHandoffs(
|
||||
typeImport.graph.nodes,
|
||||
typeImport.graph.edges,
|
||||
[],
|
||||
).flatMap((handoff) => handoff.requested_exports),
|
||||
).toEqual([]);
|
||||
|
||||
const typeReexport = await analyzeFixture(
|
||||
'export type { Extra } from "./addon.node";',
|
||||
);
|
||||
expect(typeReexport.summary.native_addon_bindings).toBe(0);
|
||||
expect(
|
||||
typeReexport.graph.nodes.filter((node) => node.kind === "native-export"),
|
||||
).toEqual([]);
|
||||
expect(
|
||||
buildJavaScriptNativeHandoffs(
|
||||
typeReexport.graph.nodes,
|
||||
typeReexport.graph.edges,
|
||||
[],
|
||||
).flatMap((handoff) => handoff.requested_exports),
|
||||
).toEqual([]);
|
||||
|
||||
const mixed = await analyzeFixture(
|
||||
'import { type OtherThing, load } from "./addon.node"; load();',
|
||||
);
|
||||
expect(mixed.summary.native_addon_bindings).toBe(1);
|
||||
const mixedExports = mixed.graph.nodes.filter(
|
||||
(node) => node.kind === "native-export",
|
||||
);
|
||||
expect(mixedExports).toHaveLength(1);
|
||||
expect(
|
||||
mixedExports[0]?.observations[0]?.properties.requested_members,
|
||||
).toEqual(["load"]);
|
||||
expect(
|
||||
buildJavaScriptNativeHandoffs(mixed.graph.nodes, mixed.graph.edges, []).map(
|
||||
(handoff) => handoff.requested_exports,
|
||||
),
|
||||
).toEqual([["load"]]);
|
||||
|
||||
const value = await analyzeFixture(
|
||||
'import { load } from "./addon.node"; load();',
|
||||
);
|
||||
expect(value.summary.native_addon_bindings).toBe(1);
|
||||
expect(
|
||||
value.graph.nodes.filter((node) => node.kind === "native-export")[0]
|
||||
?.observations[0]?.properties.requested_members,
|
||||
).toEqual(["load"]);
|
||||
}, 30_000);
|
||||
|
|
@ -2,6 +2,67 @@ import { describe, expect, it } from "vitest";
|
|||
|
||||
import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js";
|
||||
|
||||
describe("native addon type-only TypeScript imports", () => {
|
||||
it("ignores declaration-wide import type and export type from .node modules", () => {
|
||||
const analysis = analyzeJavaScriptStaticSource(`
|
||||
import type { NativeThing } from "./addon.node";
|
||||
export type { Extra } from "./addon.node";
|
||||
`);
|
||||
expect(analysis.parse_status).toBe("complete");
|
||||
expect(analysis.electron.native_addon_bindings).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps only value members from mixed type/value native imports", () => {
|
||||
const analysis = analyzeJavaScriptStaticSource(`
|
||||
import { type OtherThing, load } from "./addon.node";
|
||||
load();
|
||||
`);
|
||||
expect(analysis.electron.native_addon_bindings).toEqual([
|
||||
expect.objectContaining({
|
||||
specifier: "./addon.node",
|
||||
binding_kind: "import",
|
||||
members: ["load"],
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it("retains ordinary value imports of native addons", () => {
|
||||
const analysis = analyzeJavaScriptStaticSource(`
|
||||
import { load } from "./addon.node";
|
||||
load();
|
||||
`);
|
||||
expect(analysis.electron.native_addon_bindings).toEqual([
|
||||
expect.objectContaining({
|
||||
specifier: "./addon.node",
|
||||
binding_kind: "import",
|
||||
members: ["load"],
|
||||
}),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("native addon type-only re-exports", () => {
|
||||
it.each([
|
||||
['export { type Extra, load } from "./addon.node";', ["load"]],
|
||||
['export { type Extra } from "./addon.node";', []],
|
||||
['export type * from "./addon.node";', []],
|
||||
] as const)("retains only runtime members from %s", (source, members) => {
|
||||
const analysis = analyzeJavaScriptStaticSource(source);
|
||||
expect(analysis.parse_status).toBe("complete");
|
||||
expect(analysis.electron.native_addon_bindings).toEqual(
|
||||
members.length === 0
|
||||
? []
|
||||
: [
|
||||
expect.objectContaining({
|
||||
specifier: "./addon.node",
|
||||
binding_kind: "re-export",
|
||||
members: [...members],
|
||||
}),
|
||||
],
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("native addon loader provenance", () => {
|
||||
it("distinguishes CommonJS and ESM re-exports", () => {
|
||||
const analysis = analyzeJavaScriptStaticSource(`
|
||||
|
|
|
|||
|
|
@ -6,6 +6,12 @@ import {
|
|||
} from "../unicodeCodePointOrder.js";
|
||||
import { stripQueryAndFragment } from "../artifactPathSyntax.js";
|
||||
import type { ElectronNativeAddonBindingFinding } from "./electronStaticAnalysisTypes.js";
|
||||
import {
|
||||
isTypeOnlyExportDeclaration,
|
||||
isTypeOnlyExportSpecifier,
|
||||
isTypeOnlyImportDeclaration,
|
||||
isTypeOnlyImportSpecifier,
|
||||
} from "./javascriptModuleDeclaration.js";
|
||||
import { addLocatedFinding } from "./javascriptStaticAnalysisFindings.js";
|
||||
import {
|
||||
argumentNode,
|
||||
|
|
@ -47,7 +53,15 @@ const inspectImport = (
|
|||
context: JavaScriptFindingContext,
|
||||
): void => {
|
||||
if (!isNativeSpecifier(node.source.value, "import")) return;
|
||||
const members = node.specifiers.flatMap((specifier) => {
|
||||
// TypeScript erases `import type` and per-binding `type` imports; they never
|
||||
// request a runtime native-addon member.
|
||||
if (isTypeOnlyImportDeclaration(node)) return;
|
||||
const valueSpecifiers = node.specifiers.filter(
|
||||
(specifier) =>
|
||||
!(t.isImportSpecifier(specifier) && isTypeOnlyImportSpecifier(specifier)),
|
||||
);
|
||||
if (node.specifiers.length > 0 && valueSpecifiers.length === 0) return;
|
||||
const members = valueSpecifiers.flatMap((specifier) => {
|
||||
if (t.isImportDefaultSpecifier(specifier)) return ["default"];
|
||||
if (t.isImportNamespaceSpecifier(specifier)) return [];
|
||||
return [propertyName(specifier.imported)];
|
||||
|
|
@ -59,7 +73,7 @@ const inspectImport = (
|
|||
kind: "import",
|
||||
moduleKind: "import",
|
||||
members,
|
||||
namespaceAccess: node.specifiers.some((value) =>
|
||||
namespaceAccess: valueSpecifiers.some((value) =>
|
||||
t.isImportNamespaceSpecifier(value),
|
||||
),
|
||||
dynamicMemberAccess: false,
|
||||
|
|
@ -76,7 +90,13 @@ const inspectNamedExport = (
|
|||
!isNativeSpecifier(node.source.value, "import")
|
||||
)
|
||||
return;
|
||||
const members = node.specifiers.flatMap((specifier) => {
|
||||
if (isTypeOnlyExportDeclaration(node)) return;
|
||||
const valueSpecifiers = node.specifiers.filter(
|
||||
(specifier) =>
|
||||
!(t.isExportSpecifier(specifier) && isTypeOnlyExportSpecifier(specifier)),
|
||||
);
|
||||
if (node.specifiers.length > 0 && valueSpecifiers.length === 0) return;
|
||||
const members = valueSpecifiers.flatMap((specifier) => {
|
||||
if (t.isExportSpecifier(specifier)) return [propertyName(specifier.local)];
|
||||
if (t.isExportDefaultSpecifier(specifier)) return ["default"];
|
||||
return [];
|
||||
|
|
@ -88,7 +108,7 @@ const inspectNamedExport = (
|
|||
kind: "re-export",
|
||||
moduleKind: "import",
|
||||
members,
|
||||
namespaceAccess: node.specifiers.some((value) =>
|
||||
namespaceAccess: valueSpecifiers.some((value) =>
|
||||
t.isExportNamespaceSpecifier(value),
|
||||
),
|
||||
dynamicMemberAccess: false,
|
||||
|
|
@ -101,7 +121,8 @@ const inspectExportAll = (
|
|||
): void => {
|
||||
if (
|
||||
node.source === undefined ||
|
||||
!isNativeSpecifier(node.source.value, "import")
|
||||
!isNativeSpecifier(node.source.value, "import") ||
|
||||
isTypeOnlyExportDeclaration(node)
|
||||
)
|
||||
return;
|
||||
addBinding({
|
||||
|
|
|
|||
|
|
@ -147,8 +147,8 @@ const registerInspectorSocket = (
|
|||
socket.close();
|
||||
return;
|
||||
}
|
||||
socket.send(JSON.stringify({ id: command.id, result: {} }));
|
||||
respondToInspectorCommand(socket, command, options);
|
||||
socket.send(JSON.stringify({ id: command.id, result: {} }));
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue