Skip to content

Commit ea38fd6

Browse files
committed
Limit the build.os and build.arch replacements to scopes that can import deno_ral
The map covered both APIs in every scope, so a Deno.build.os added to the engine extension template (scope resources, user code) or to a plain JavaScript file (src-js) failed CI and asked for src/deno_ral/platform.ts, which neither can import. They are now errors only in src, package/src and tests; elsewhere the finding is the uncovered warning.
1 parent b762011 commit ea38fd6

3 files changed

Lines changed: 34 additions & 1 deletion

File tree

‎dev-docs/runtime-migration/02c-ratchet-pr-delta-and-guidance.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,3 +445,8 @@ Run from the branch worktree; `Q` is `package/dist/bin/quarto` (`quarto.cmd` on
445445
- Decided with Chris: option "path list, warning only". `diffChanges` no longer attaches a replacement (so no error) to a finding under `kBrowserSharedPaths` (`src/core/lib/`, the code the YAML-intelligence web worker imports, which cannot import `src/deno_ral/`); it falls under the uncovered warning text. The constant is module-private (no new export); PR 19 removes the two existing `Deno.*` uses there.
446446
- Test `a covered API in browser-shared code is a warning, not an error` failed before the change (assertion: the finding carried the `build.os` replacement) and passes after; `unit/tools/check-runtime-boundary.test.ts` → 37 passed. Counts unchanged (994/49, 7/0, 10/1, 240/3, 832/177).
447447
- `_TEMPLATE.md` gets one sentence on `kBrowserSharedPaths` next to the map rule. Deviation from the plan's file list: the constant and template sentence were not in the plan.
448+
449+
### 2026-10-22: second external review (dd4a49b34)
450+
451+
- P2 verified and fixed: `build.os` and `build.arch` were covered in every scope, so adding one to the engine template under `src/resources/create/` (scope `resources`, user code that cannot import `src/deno_ral/`) or to a plain `.js` file (`src-js`) failed CI with a hint that cannot work there. Both map entries now carry `scopes: ["src", "package/src", "tests"]`, the scopes that import `deno_ral` today (`package/src` and `tests` verified by grep). Test `a covered API is only an error where deno_ral can be imported` failed before (`resources` returned the hint), passes after; `unit/tools/check-runtime-boundary.test.ts` → 38 passed, counts unchanged.
452+
- Its note about a concurrent uncommitted change in `src/core/cleanup.ts` was a probe edit of mine from a failed reproduction attempt, reverted with `git checkout -- <file>` after checking the diff was that one line; the tree is clean.

‎tests/unit/tools/check-runtime-boundary.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,3 +675,23 @@ unitTest("check-runtime-boundary - a covered API in browser-shared code is a war
675675
assertEquals(rendered.errors.map((e) => e.file), ["src/core/other.ts"]);
676676
assertEquals(rendered.warnings.map((w) => w.file), ["src/core/lib/errors.ts"]);
677677
});
678+
679+
// deno-lint-ignore require-await
680+
unitTest("check-runtime-boundary - a covered API is only an error where deno_ral can be imported", async () => {
681+
const hint = kRalDenoReplacements["build.os"].hint;
682+
for (const scope of ["src", "package/src", "tests"]) {
683+
assertEquals(denoReplacement("build.os", scope), hint, scope);
684+
assertEquals(denoReplacement("build.arch", scope), kRalDenoReplacements["build.arch"].hint, scope);
685+
}
686+
for (const scope of ["resources", "src-js"]) {
687+
assertEquals(denoReplacement("build.os", scope), undefined, scope);
688+
assertEquals(denoReplacement("build.arch", scope), undefined, scope);
689+
}
690+
691+
const report = diffChanges([
692+
{ path: "src/resources/create/extensions/engine/a.ts", before: "", after: `export const o = ${DENO}build.os;\n`, addedLines: [1] },
693+
]);
694+
assertEquals(report.findings.length, 1);
695+
assertEquals(report.findings[0].scope, "resources");
696+
assertEquals(report.findings[0].replacement, undefined);
697+
});

‎tools/check-runtime-boundary.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,14 +296,22 @@ export function acceptIncrease(current: Baseline): Baseline {
296296
);
297297
}
298298

299+
/** The scopes whose code can import src/deno_ral/. `resources` holds user-script and extension templates and
300+
* `src-js` plain JavaScript, neither of which can. */
301+
const kDenoRalImportScopes: readonly string[] = ["src", "package/src", "tests"];
302+
299303
/** Keyed by exact API key. A PR that adds a deno_ral replacement adds its keys here. */
300304
export const kRalDenoReplacements: Readonly<
301305
Record<string, { hint: string; scopes?: readonly string[] }>
302306
> = {
303307
"build.os": {
304308
hint: "os (or isWindows, isMac, isLinux) from src/deno_ral/platform.ts",
309+
scopes: kDenoRalImportScopes,
310+
},
311+
"build.arch": {
312+
hint: "arch from src/deno_ral/platform.ts",
313+
scopes: kDenoRalImportScopes,
305314
},
306-
"build.arch": { hint: "arch from src/deno_ral/platform.ts" },
307315
};
308316

309317
/** Keyed by stdModuleOf(). `module` is repository-relative. A suggestion only: never turns a finding into an error. */

0 commit comments

Comments
 (0)