Skip to content

Commit b762011

Browse files
committed
Report a covered API in browser-shared code as a warning
src/core/lib is imported by the browser-side YAML intelligence, so it cannot import src/deno_ral/platform.ts (it probes Deno in a try block instead). A new Deno.build.os there got the blocking error with a deno_ral hint that cannot compile in that code, and --accept-increase cannot clear a finding. Findings under src/core/lib/ now use the uncovered warning text.
1 parent a16205a commit b762011

4 files changed

Lines changed: 32 additions & 2 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,3 +439,9 @@ Run from the branch worktree; `Q` is `package/dist/bin/quarto` (`quarto.cmd` on
439439
- Fix: delta mode passes `renderDelta` the baseline committed at the `--against` revision, read by the new exported `baselineAt` (`git cat-file -e` to detect an absent file, fallback to the working-tree baseline for a revision from before the ratchet; a malformed baseline exits 2). New export, not in the plan: needed to unit-test the logic with a real temp git repository. After: probe PR prints no notice, V3 still prints `main is above … (tests Deno.* calls 832 > 826)`, `unit/tools/check-runtime-boundary.test.ts` → 36 passed.
440440
- Correction to the earlier note on the `execProcess` decoder: counts are not always unaffected. A corrupted path from `git diff --name-only` or the untracked list fails both the `ls-tree` membership check and the disk read, so the file contributes nothing and a covered increase in it would be missed silently; only a corrupted `ls-tree` entry over-reports. Still 0 tracked non-ASCII paths, and the fix stays a separate bug on main (backported to v1.10).
441441
- Open, needs a decision: a covered `Deno.build.os` added under `src/core/lib/` (browser-shared: the YAML-intelligence web worker imports it, so it cannot import `src/deno_ral/platform.ts`) gets a blocking error with a hint that cannot compile there. PR 19 plans a runtime-neutral probe for the two existing uses (`errors.ts`, `colors.ts`).
442+
443+
### 2026-10-22: browser-shared code
444+
445+
- 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.
446+
- 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).
447+
- `_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.

‎dev-docs/runtime-migration/_TEMPLATE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Repository conventions that plans must respect (`.claude/CLAUDE.md` and `.claude
8484
- Unit tests live in `tests/unit/` (runtime-layer tests under the existing `tests/unit/ral/`), use `unitTest` from `tests/test.ts` and assertions from `tests/asserts.ts` (added by PR 02), and run with `cd tests && QUARTO_TESTS_NO_CONFIG=true ./run-tests.sh --agent unit/<file>.test.ts`.
8585
The ratchet (PR 02) has a `tests` scope: tests written by PRs 09 and later use `deno_ral` `fs`/`temp`/`process`, never raw `Deno.*`.
8686
- Landing order inside Phase 1: 02 → 02b → 08 → 22 → 10–13 → 14, with 09 before 10; 22 depends on PR 04 (Phase 0). 02c lands before any other count-changing PR.
87-
- A PR that adds an equivalent `deno_ral` replacement for a `Deno.*` API adds its exact keys (e.g. `readTextFileSync`) to `kRalDenoReplacements` in `tools/check-runtime-boundary.ts` in the same change, which turns later increases of that API into errors; a PR that adds a std seam may add a `kStdSuggestions` entry (suggestions never fail a PR).
87+
- A PR that adds an equivalent `deno_ral` replacement for a `Deno.*` API adds its exact keys (e.g. `readTextFileSync`) to `kRalDenoReplacements` in `tools/check-runtime-boundary.ts` in the same change, which turns later increases of that API into errors; a PR that adds a std seam may add a `kStdSuggestions` entry (suggestions never fail a PR). Under `kBrowserSharedPaths` (`src/core/lib/`, which the browser-side YAML intelligence imports and which therefore cannot import `deno_ral`) a covered API is only a warning.
8888
A conflict in a codemod PR is resolved by resetting the file to `main` and re-running the codemod, never by hand (the codemod is idempotent, PR 09); a conflict on a shared file (`tools/runtime-boundary-baseline.json`, `deno.lock`, the shim, the `deno-ral.md` table, a changelog) follows README ground rule 8.
8989
- Commands in the plans write the Linux path `package/dist/bin/tools/x86_64/deno`; on Apple Silicon it is `tools/aarch64/deno`, on Windows `tools\x86_64\deno.exe`.
9090
- Size is by file count (S < 10, M 10–40, L > 40 files); a plan that is large by volume but small by files says so next to the letter.

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,3 +660,18 @@ unitTest("check-runtime-boundary - baselineAt reads the baseline committed at th
660660
temp.cleanup();
661661
}
662662
});
663+
664+
// deno-lint-ignore require-await
665+
unitTest("check-runtime-boundary - a covered API in browser-shared code is a warning, not an error", async () => {
666+
const report = diffChanges([
667+
{ path: "src/core/lib/errors.ts", before: "", after: `export const o = ${DENO}build.os;\n`, addedLines: [1] },
668+
{ path: "src/core/other.ts", before: "", after: `export const o = ${DENO}build.os;\n`, addedLines: [1] },
669+
]);
670+
const byFile = Object.fromEntries(report.findings.map((f) => [f.file, f]));
671+
assertEquals(byFile["src/core/lib/errors.ts"].replacement, undefined);
672+
assertEquals(byFile["src/core/other.ts"].replacement, kRalDenoReplacements["build.os"].hint);
673+
674+
const rendered = renderDelta(report, { "src": { denoCalls: 996, stdImportFiles: 49 } }, { "src": { denoCalls: 994, stdImportFiles: 49 } });
675+
assertEquals(rendered.errors.map((e) => e.file), ["src/core/other.ts"]);
676+
assertEquals(rendered.warnings.map((w) => w.file), ["src/core/lib/errors.ts"]);
677+
});

‎tools/check-runtime-boundary.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,13 @@ export const kStdSuggestions: Readonly<
316316
"assert": { module: "tests/asserts.ts", scopes: ["tests"] },
317317
};
318318

319+
/** Code that also runs in the browser (the YAML intelligence web worker imports it), so it cannot
320+
* import src/deno_ral/: a covered API there is only ever a warning. */
321+
const kBrowserSharedPaths: readonly string[] = ["src/core/lib/"];
322+
323+
const isBrowserShared = (path: string) =>
324+
kBrowserSharedPaths.some((prefix) => path.startsWith(prefix));
325+
319326
export function denoReplacement(api: string, scope: string): string | undefined {
320327
const entry = Object.hasOwn(kRalDenoReplacements, api)
321328
? kRalDenoReplacements[api]
@@ -419,7 +426,9 @@ export function diffChanges(changes: FileChange[]): DeltaReport {
419426
};
420427
const line = firstAddedLine(e.change, "Deno." + api);
421428
if (line !== undefined) finding.line = line;
422-
const replacement = denoReplacement(api, scope);
429+
const replacement = isBrowserShared(e.change.path)
430+
? undefined
431+
: denoReplacement(api, scope);
423432
if (replacement !== undefined) finding.replacement = replacement;
424433
findings.push(finding);
425434
}

0 commit comments

Comments
 (0)