Skip to content

Commit 1f0befd

Browse files
fix: align napi declaration nullability
1 parent b31be6c commit 1f0befd

5 files changed

Lines changed: 111 additions & 5 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { readFile } from "node:fs/promises";
2+
import { pathToFileURL } from "node:url";
3+
4+
const SELECTED_METHODS = ["source", "name"];
5+
const ROOT_URL = new URL("../../", import.meta.url);
6+
const GENERATED_DECLARATION_URL = new URL("target/napi-sourcemap.d.ts", ROOT_URL);
7+
const PUBLIC_DECLARATION_URL = new URL("packages/sourcemap/index.d.ts", ROOT_URL);
8+
9+
const extractReturnType = (declaration, method) => {
10+
const signature = new RegExp(
11+
`^\\s*${method}\\s*\\(\\s*index\\s*:\\s*number\\s*\\)\\s*:\\s*([^;\\n]+)\\s*;?`,
12+
"m",
13+
).exec(declaration);
14+
15+
return signature?.[1].trim().replaceAll(/\s+/g, " ") ?? null;
16+
};
17+
18+
/** Compare selected generated and public NAPI method return types. */
19+
export const findDeclarationMismatches = (generatedDeclaration, publicDeclaration) => {
20+
const mismatches = [];
21+
22+
for (const method of SELECTED_METHODS) {
23+
const generatedReturnType = extractReturnType(generatedDeclaration, method);
24+
const publicReturnType = extractReturnType(publicDeclaration, method);
25+
26+
if (generatedReturnType === publicReturnType && generatedReturnType !== null) {
27+
continue;
28+
}
29+
30+
mismatches.push(
31+
`${method}: generated returns ${generatedReturnType ?? "no declaration"}, public declaration returns ${publicReturnType ?? "no declaration"}`,
32+
);
33+
}
34+
35+
return mismatches;
36+
};
37+
38+
const main = async () => {
39+
const [generatedDeclaration, publicDeclaration] = await Promise.all([
40+
readFile(GENERATED_DECLARATION_URL, "utf8"),
41+
readFile(PUBLIC_DECLARATION_URL, "utf8"),
42+
]);
43+
const mismatches = findDeclarationMismatches(generatedDeclaration, publicDeclaration);
44+
45+
if (mismatches.length === 0) {
46+
return;
47+
}
48+
49+
throw new Error(`NAPI declaration drift detected:\n${mismatches.join("\n")}`);
50+
};
51+
52+
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
53+
await main();
54+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import assert from "node:assert/strict";
2+
import { readFile } from "node:fs/promises";
3+
import { describe, it } from "node:test";
4+
5+
import { findDeclarationMismatches } from "./check-napi-declarations.mjs";
6+
7+
const generatedDeclaration = `
8+
export declare class SourceMap {
9+
source(index: number): string | null
10+
name(index: number): string | null
11+
}
12+
`;
13+
14+
describe("findDeclarationMismatches", () => {
15+
it("accepts matching selected method signatures", () => {
16+
const mismatches = findDeclarationMismatches(generatedDeclaration, generatedDeclaration);
17+
assert.deepEqual(mismatches, []);
18+
});
19+
20+
it("reports a mismatching selected method signature", () => {
21+
const publicDeclaration = generatedDeclaration.replace(
22+
"source(index: number): string | null",
23+
"source(index: number): string",
24+
);
25+
26+
const mismatches = findDeclarationMismatches(generatedDeclaration, publicDeclaration);
27+
28+
assert.deepEqual(mismatches, [
29+
"source: generated returns string | null, public declaration returns string",
30+
]);
31+
});
32+
33+
it("keeps the public declaration aligned with generated nullability", async () => {
34+
const publicDeclaration = await readFile(
35+
new URL("../../packages/sourcemap/index.d.ts", import.meta.url),
36+
"utf8",
37+
);
38+
39+
assert.deepEqual(findDeclarationMismatches(generatedDeclaration, publicDeclaration), []);
40+
});
41+
});

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"analyze:js": "pnpm run analyze:js:dupes && pnpm run analyze:js:health",
1818
"analyze:js:dupes": "fallow dupes --format json --quiet",
1919
"analyze:js:health": "fallow health --format json --quiet --top 20",
20+
"check:napi-declarations": "pnpm --filter @srcmap/sourcemap exec napi build --release --platform --no-js --dts ../../target/napi-sourcemap.d.ts && node .github/scripts/check-napi-declarations.mjs",
2021
"check": "pnpm run fmt:check && pnpm run lint:rust && pnpm run lint:js && pnpm run typos && pnpm run deny",
2122
"fmt:check": "cargo fmt --all --check && pnpm run fmt:js:check",
2223
"fmt:js": "oxfmt --write --no-error-on-unmatched-pattern \"package.json\" \"benchmarks/**/*.mjs\" \"benchmarks/**/*.json\" \"packages/**/*.js\" \"packages/**/*.cjs\" \"packages/**/*.mjs\" \"packages/**/*.d.ts\" \"packages/**/*.json\"",
@@ -28,7 +29,7 @@
2829
"typos": "typos",
2930
"deny": "cargo deny check",
3031
"test": "pnpm run test:rust && pnpm run test:js",
31-
"test:js": "node --test packages/codec/__tests__/codec.test.mjs packages/sourcemap/__tests__/sourcemap.test.mjs packages/sourcemap-wasm/__tests__/sourcemap-wasm.test.mjs packages/sourcemap-wasm/__tests__/coverage-utils.test.mjs packages/sourcemap-wasm/__tests__/browser.test.mjs packages/generator-wasm/__tests__/generator-wasm.test.mjs packages/remapping-wasm/__tests__/remapping-wasm.test.mjs packages/trace-mapping/__tests__/trace-mapping.test.mjs packages/trace-mapping/__tests__/compat.test.mjs packages/source-map/__tests__/source-map.test.mjs packages/gen-mapping/__tests__/gen-mapping.test.mjs packages/gen-mapping/__tests__/gen-mapping.cjs.test.cjs packages/remapping/__tests__/remapping.test.mjs packages/remapping/__tests__/remapping.cjs.test.cjs packages/remapping/__tests__/compat.test.mjs",
32+
"test:js": "node --test .github/scripts/check-napi-declarations.test.mjs packages/codec/__tests__/codec.test.mjs packages/sourcemap/__tests__/sourcemap.test.mjs packages/sourcemap-wasm/__tests__/sourcemap-wasm.test.mjs packages/sourcemap-wasm/__tests__/coverage-utils.test.mjs packages/sourcemap-wasm/__tests__/browser.test.mjs packages/generator-wasm/__tests__/generator-wasm.test.mjs packages/remapping-wasm/__tests__/remapping-wasm.test.mjs packages/trace-mapping/__tests__/trace-mapping.test.mjs packages/trace-mapping/__tests__/compat.test.mjs packages/source-map/__tests__/source-map.test.mjs packages/gen-mapping/__tests__/gen-mapping.test.mjs packages/gen-mapping/__tests__/gen-mapping.cjs.test.cjs packages/remapping/__tests__/remapping.test.mjs packages/remapping/__tests__/remapping.cjs.test.cjs packages/remapping/__tests__/compat.test.mjs",
3233
"test:rust": "cargo test",
3334
"coverage": "pnpm run coverage:rust && pnpm run coverage:js",
3435
"coverage:js": "mkdir -p coverage && node --test --experimental-test-coverage --test-reporter=lcov --test-reporter-destination=coverage/js-lcov.info --test-reporter=spec --test-reporter-destination=stdout packages/codec/__tests__/codec.test.mjs packages/sourcemap/__tests__/sourcemap.test.mjs packages/sourcemap-wasm/__tests__/sourcemap-wasm.test.mjs packages/sourcemap-wasm/__tests__/coverage-utils.test.mjs packages/generator-wasm/__tests__/generator-wasm.test.mjs packages/remapping-wasm/__tests__/remapping-wasm.test.mjs packages/trace-mapping/__tests__/trace-mapping.test.mjs packages/trace-mapping/__tests__/compat.test.mjs packages/source-map/__tests__/source-map.test.mjs packages/gen-mapping/__tests__/gen-mapping.test.mjs packages/gen-mapping/__tests__/gen-mapping.cjs.test.cjs packages/remapping/__tests__/remapping.test.mjs packages/remapping/__tests__/remapping.cjs.test.cjs packages/remapping/__tests__/compat.test.mjs",

packages/sourcemap/__tests__/sourcemap.test.mjs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ describe("sources and names", () => {
4646
const sm = new SourceMap(MULTI_SOURCE_MAP);
4747
assert.deepEqual(sm.sources, ["a.js", "b.js"]);
4848
});
49+
50+
it("returns null for an out-of-range source index", () => {
51+
const sm = new SourceMap(SIMPLE_MAP);
52+
assert.equal(sm.source(1), null);
53+
});
54+
55+
it("returns null for an out-of-range name index", () => {
56+
const sm = new SourceMap(SIMPLE_MAP);
57+
assert.equal(sm.name(2), null);
58+
});
4959
});
5060

5161
describe("mappingCount and lineCount", () => {

packages/sourcemap/index.d.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,17 +133,17 @@ export declare class SourceMap {
133133
* Resolve a source index to a source filename.
134134
*
135135
* @param index - Source index from a lookup result
136-
* @returns The source filename
136+
* @returns The source filename, or `null` if the index is out of range
137137
*/
138-
source(index: number): string;
138+
source(index: number): string | null;
139139

140140
/**
141141
* Resolve a name index to a name string.
142142
*
143143
* @param index - Name index from a lookup result
144-
* @returns The name string
144+
* @returns The name string, or `null` if the index is out of range
145145
*/
146-
name(index: number): string;
146+
name(index: number): string | null;
147147

148148
/** All source filenames in the source map. */
149149
readonly sources: string[];

0 commit comments

Comments
 (0)