From f437c615a03ddba146f90f9645a193333fba946a Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 10 Nov 2026 12:56:36 +0400 Subject: [PATCH] Add a --write mode to the package boundary check `node scripts/check-package-boundaries.mjs --write` regenerates each group's module list in scripts/package-boundaries.json from what the group's build reaches: entries that own nothing reached are removed, newly reached modules are appended in sorted order, and the order of kept entries stays. The check without the flag is unchanged. --- docs/CODE-BOUNDARIES.md | 6 + scripts/check-package-boundaries.mjs | 98 ++++++++++-- src/tooling/packageBoundaries.test.mjs | 209 ++++++++++++++++++++++++- 3 files changed, 293 insertions(+), 20 deletions(-) diff --git a/docs/CODE-BOUNDARIES.md b/docs/CODE-BOUNDARIES.md index 292ab077c..794b9da0f 100644 --- a/docs/CODE-BOUNDARIES.md +++ b/docs/CODE-BOUNDARIES.md @@ -59,6 +59,12 @@ See [application construction](APPLICATION.md) and the file under that directory. Unused imports still count. Node exports have only a `node` condition; browser groups cannot use build-only dependency exceptions. + Run `node scripts/check-package-boundaries.mjs --write` to regenerate each + group's `modules` list in `scripts/package-boundaries.json`, retaining useful + entries in order and appending newly reached paths in sorted order. The check + without `--write` is unchanged and does not reject stale entries. Review the + manifest diff like a lockfile diff before accepting changes to ownership. + Portable source graphs cannot reach application/rendering, Node, Cesium or browser globals. This includes `sources/*`, dedicated `layers/*/source` exports, flight/military/vessel record and ingestion exports, action schemas, the session diff --git a/scripts/check-package-boundaries.mjs b/scripts/check-package-boundaries.mjs index f5c92763e..71d076229 100644 --- a/scripts/check-package-boundaries.mjs +++ b/scripts/check-package-boundaries.mjs @@ -1,19 +1,28 @@ -import { readFile, realpath } from 'node:fs/promises'; +import { readFile, realpath, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import vue from '@vitejs/plugin-vue'; import { VUE_PLUGIN_OPTIONS } from '../build/vueOptions.js'; import { build, normalizePath } from 'vite'; +import * as prettier from 'prettier'; /** Build each browser or Node export group and reject imports outside its declared ownership. */ export async function checkPackageBoundaries(root) { + return buildPackageBoundaries(root, false); +} + +/** Regenerate module ownership from each export group's complete build graph. */ +export async function writePackageBoundaries(root) { + return buildPackageBoundaries(root, true); +} + +async function buildPackageBoundaries(root, write) { root = await realpath(root); const readJson = async (name) => JSON.parse(await readFile(path.join(root, name), 'utf8')); const pkg = await readJson('package.json'); - const { engineImportExceptions, ...groups } = await readJson( - 'scripts/package-boundaries.json', - ); + const manifest = await readJson('scripts/package-boundaries.json'); + const { engineImportExceptions, ...groups } = manifest; const declaredExports = Object.keys(pkg.exports || {}).sort(); const classifiedExports = Object.values(groups) .flatMap((group) => group.exports) @@ -34,7 +43,7 @@ export async function checkPackageBoundaries(root) { } if ( !Array.isArray(group.modules) || - !group.modules.length || + (!write && !group.modules.length) || !Array.isArray(group.external) ) { throw new Error(`Invalid package boundary: ${name}`); @@ -42,6 +51,7 @@ export async function checkPackageBoundaries(root) { // A module entry ending in "/" owns every file under that directory. const allowed = new Set(); const allowedDirectories = []; + const modulePaths = new Map(); const owns = (id) => allowed.has(id) || allowedDirectories.some((dir) => id.startsWith(dir)); for (const module of group.modules) { @@ -53,7 +63,13 @@ export async function checkPackageBoundaries(root) { ) { throw new Error(`Boundary modules must be repository paths: ${name}`); } - const resolved = await realpath(path.join(root, module)); + let resolved; + try { + resolved = await realpath(path.join(root, module)); + } catch (error) { + if (write && error.code === 'ENOENT') continue; + throw error; + } const relative = path.relative(root, resolved); if ( relative === '..' || @@ -67,6 +83,7 @@ export async function checkPackageBoundaries(root) { } else { allowed.add(normalizePath(resolved)); } + modulePaths.set(module, normalizePath(resolved)); } for (const external of group.external) { if ( @@ -92,7 +109,7 @@ export async function checkPackageBoundaries(root) { if ( typeof target !== 'string' || !target.startsWith('./') || - !owns(normalizePath(path.resolve(root, target))) + (!write && !owns(normalizePath(path.resolve(root, target)))) ) { throw new Error(`Export must point to an owned module: ${key}`); } @@ -119,7 +136,7 @@ export async function checkPackageBoundaries(root) { // template blocks belong to the component file itself. if (info.id.startsWith('\0')) return; const id = info.id.split('?')[0]; - if (!owns(id)) { + if (!write && !owns(id)) { throw new Error( `Package boundary ${name} imports an unowned module: ${path.relative(root, id)}`, ); @@ -148,7 +165,44 @@ export async function checkPackageBoundaries(root) { }, }, }); - reports.push({ name, exports: input.length, modules: seen.size }); + if (write) { + const reached = [...seen]; + const kept = group.modules.filter((module) => { + const resolved = modulePaths.get(module); + return module.endsWith('/') + ? reached.some((id) => id.startsWith(`${resolved}/`)) + : seen.has(resolved); + }); + const removed = group.modules.filter((module) => !kept.includes(module)); + const added = reached + .filter((id) => !owns(id)) + .map((id) => { + const relative = path.relative(root, id); + if ( + !path.isAbsolute(id) || + relative === '..' || + relative.startsWith(`..${path.sep}`) || + path.isAbsolute(relative) + ) { + throw new Error(`Boundary module escapes repository: ${name}`); + } + return normalizePath(relative); + }) + .sort(); + group.modules = [...kept, ...added]; + if (added.length || removed.length) + reports.push({ name, added, removed }); + } else { + reports.push({ name, exports: input.length, modules: seen.size }); + } + } + if (write && reports.length) { + const file = path.join(root, 'scripts/package-boundaries.json'); + const formatted = await prettier.format(JSON.stringify(manifest), { + ...(await prettier.resolveConfig(file)), + filepath: file, + }); + await writeFile(file, formatted); } return reports; } @@ -158,13 +212,25 @@ const invoked = process.argv[1] : ''; if (import.meta.url === invoked) { try { - const reports = await checkPackageBoundaries( - fileURLToPath(new URL('../', import.meta.url)), - ); - for (const report of reports) - console.log( - `Checked ${report.name}: ${report.exports} exports, ${report.modules} owned modules.`, - ); + const root = fileURLToPath(new URL('../', import.meta.url)); + if (process.argv.includes('--write')) { + const reports = await writePackageBoundaries(root); + for (const { name, added, removed } of reports) { + console.log( + `Updated ${name}: ${added.length} added, ${removed.length} removed.`, + ); + if (added.length + removed.length <= 20) { + for (const module of added) console.log(` + ${module}`); + for (const module of removed) console.log(` - ${module}`); + } + } + } else { + const reports = await checkPackageBoundaries(root); + for (const report of reports) + console.log( + `Checked ${report.name}: ${report.exports} exports, ${report.modules} owned modules.`, + ); + } } catch (error) { console.error(error.message); process.exitCode = 1; diff --git a/src/tooling/packageBoundaries.test.mjs b/src/tooling/packageBoundaries.test.mjs index 3cee9fd32..c823448c8 100644 --- a/src/tooling/packageBoundaries.test.mjs +++ b/src/tooling/packageBoundaries.test.mjs @@ -1,16 +1,29 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtemp, mkdir, writeFile, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; +import { execFileSync } from 'node:child_process'; +import { + copyFile, + mkdtemp, + mkdir, + readFile, + writeFile, + rm, +} from 'node:fs/promises'; import path from 'node:path'; -import { checkPackageBoundaries } from '../../scripts/check-package-boundaries.mjs'; +import { fileURLToPath } from 'node:url'; +import { + checkPackageBoundaries, + writePackageBoundaries, +} from '../../scripts/check-package-boundaries.mjs'; + +const repoRoot = fileURLToPath(new URL('../../', import.meta.url)); async function fixture( t, entry = 'export const value = 1;', extraExports = {}, ) { - const root = await mkdtemp(path.join(tmpdir(), 'gev-boundaries-')); + const root = await mkdtemp(path.join(repoRoot, 'tmp_package_boundaries_')); t.after(() => rm(root, { recursive: true, force: true })); await mkdir(path.join(root, 'scripts')); await writeFile( @@ -162,3 +175,191 @@ test('a module entry ending in a slash owns every file under that directory', as /unowned module.*shared\.js/, ); }); + +test('regeneration adds unowned modules, including unused and dynamic imports', async (t) => { + const root = await fixture( + t, + "import { unused } from './unused.js'; export const load = () => import('./lazy.js');", + ); + await writeFile(path.join(root, 'unused.js'), 'export const unused = 1;'); + await writeFile(path.join(root, 'lazy.js'), 'export const lazy = 2;'); + assert.deepEqual(await writePackageBoundaries(root), [ + { name: 'feature', added: ['lazy.js', 'unused.js'], removed: [] }, + ]); + const manifest = JSON.parse( + await readFile(path.join(root, 'scripts/package-boundaries.json'), 'utf8'), + ); + assert.deepEqual(manifest.feature.modules, [ + 'entry.js', + 'lazy.js', + 'unused.js', + ]); + assert.deepEqual(await checkPackageBoundaries(root), [ + { name: 'feature', exports: 1, modules: 3 }, + ]); +}); + +test('regeneration removes unreached files without making the check reject stale entries', async (t) => { + const root = await fixture(t); + const file = path.join(root, 'scripts/package-boundaries.json'); + const manifest = JSON.parse(await readFile(file, 'utf8')); + manifest.feature.modules.push('stale.js'); + await writeFile(path.join(root, 'stale.js'), 'export const stale = 1;'); + await writeFile(file, JSON.stringify(manifest)); + assert.deepEqual(await checkPackageBoundaries(root), [ + { name: 'feature', exports: 1, modules: 1 }, + ]); + assert.deepEqual(await writePackageBoundaries(root), [ + { name: 'feature', added: [], removed: ['stale.js'] }, + ]); + assert.deepEqual(JSON.parse(await readFile(file, 'utf8')).feature.modules, [ + 'entry.js', + ]); +}); + +test('regeneration removes deleted entries and adds an unowned export entry', async (t) => { + const root = await fixture(t); + const file = path.join(root, 'scripts/package-boundaries.json'); + const manifest = JSON.parse(await readFile(file, 'utf8')); + manifest.feature.modules = ['deleted.js', 'deleted/']; + await writeFile(file, JSON.stringify(manifest)); + assert.deepEqual(await writePackageBoundaries(root), [ + { + name: 'feature', + added: ['entry.js'], + removed: ['deleted.js', 'deleted/'], + }, + ]); + assert.deepEqual(JSON.parse(await readFile(file, 'utf8')).feature.modules, [ + 'entry.js', + ]); +}); + +test('regeneration keeps a directory only while it owns a reached module', async (t) => { + const root = await fixture( + t, + "export { value } from './shared/nested/value.js'; export { other } from './shared.js';", + ); + await mkdir(path.join(root, 'shared/nested'), { recursive: true }); + await writeFile( + path.join(root, 'shared/nested/value.js'), + 'export const value = 1;', + ); + await writeFile(path.join(root, 'shared.js'), 'export const other = 2;'); + const file = path.join(root, 'scripts/package-boundaries.json'); + const manifest = JSON.parse(await readFile(file, 'utf8')); + manifest.feature.modules.push('shared/'); + await writeFile(file, JSON.stringify(manifest)); + assert.deepEqual(await writePackageBoundaries(root), [ + { name: 'feature', added: ['shared.js'], removed: [] }, + ]); + assert.deepEqual(JSON.parse(await readFile(file, 'utf8')).feature.modules, [ + 'entry.js', + 'shared/', + 'shared.js', + ]); + await writeFile( + path.join(root, 'entry.js'), + "export { other } from './shared.js';", + ); + assert.deepEqual(await writePackageBoundaries(root), [ + { name: 'feature', added: [], removed: ['shared/'] }, + ]); + assert.deepEqual(JSON.parse(await readFile(file, 'utf8')).feature.modules, [ + 'entry.js', + 'shared.js', + ]); +}); + +test('regeneration preserves kept entry order and all other manifest fields', async (t) => { + const root = await fixture( + t, + "import './z.js'; import './b.js'; import './a.js'; export const value = 1;", + { './build': { node: './build.js' } }, + ); + for (const name of ['z.js', 'b.js', 'a.js', 'build.js']) { + await writeFile(path.join(root, name), 'export const value = 1;'); + } + const file = path.join(root, 'scripts/package-boundaries.json'); + const manifest = { + feature: { + exports: ['./feature'], + modules: ['z.js', 'entry.js'], + external: [], + }, + engineImportExceptions: [ + { file: 'entry.js', package: 'example', reason: 'Fixture exception.' }, + ], + build: { + runtime: 'node', + exports: ['./build'], + modules: ['build.js'], + external: [], + }, + }; + await writeFile(file, JSON.stringify(manifest)); + assert.deepEqual(await writePackageBoundaries(root), [ + { name: 'feature', added: ['a.js', 'b.js'], removed: [] }, + ]); + manifest.feature.modules.push('a.js', 'b.js'); + const updated = JSON.parse(await readFile(file, 'utf8')); + assert.deepEqual(updated, manifest); + assert.deepEqual(Object.keys(updated), Object.keys(manifest)); +}); + +test('a failed regeneration leaves the manifest unchanged', async (t) => { + const root = await fixture(t, undefined, { './broken': './missing.js' }); + const file = path.join(root, 'scripts/package-boundaries.json'); + const manifest = JSON.parse(await readFile(file, 'utf8')); + manifest.feature.modules = []; + manifest.broken = { exports: ['./broken'], modules: [], external: [] }; + const original = JSON.stringify(manifest); + await writeFile(file, original); + await assert.rejects(writePackageBoundaries(root), /missing\.js/); + assert.equal(await readFile(file, 'utf8'), original); +}); + +test('the CLI reports changes and a second --write leaves the manifest untouched', async (t) => { + const root = await fixture(t, "export { value } from './value.js';"); + await writeFile(path.join(root, 'value.js'), 'export const value = 1;'); + await writeFile(path.join(root, 'stale.js'), 'export const stale = 1;'); + const file = path.join(root, 'scripts/package-boundaries.json'); + const manifest = JSON.parse(await readFile(file, 'utf8')); + manifest.feature.modules.push('stale.js'); + await writeFile(file, JSON.stringify(manifest)); + await mkdir(path.join(root, 'build')); + for (const name of [ + 'scripts/check-package-boundaries.mjs', + 'build/vueOptions.js', + ]) { + await copyFile(path.join(repoRoot, name), path.join(root, name)); + } + const run = (args) => + execFileSync( + process.execPath, + ['scripts/check-package-boundaries.mjs', ...args], + { + cwd: root, + encoding: 'utf8', + env: { ...process.env, GOOGLE_MAPS_API_KEY: '', CESIUM_ION_TOKEN: '' }, + }, + ); + assert.equal( + run(['--write']), + 'Updated feature: 1 added, 1 removed.\n + value.js\n - stale.js\n', + ); + const first = await readFile(file, 'utf8'); + assert.ok(first.includes('"exports": ["./feature"]')); + assert.ok(first.endsWith('\n')); + assert.equal(run(['--write']), ''); + assert.equal(await readFile(file, 'utf8'), first); + assert.equal(run([]), 'Checked feature: 1 exports, 2 owned modules.\n'); + + let entry = "export { value } from './value.js';"; + for (let i = 0; i < 21; i++) { + await writeFile(path.join(root, `extra${i}.js`), 'export const extra = 1;'); + entry += `\nimport './extra${i}.js';`; + } + await writeFile(path.join(root, 'entry.js'), entry); + assert.equal(run(['--write']), 'Updated feature: 21 added, 0 removed.\n'); +});