mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-11 13:52:19 +00:00
Own shared foundations by directory and check the manifest both ways
src/i18n/, src/geo/ and the engine-neutral part of src/globe/ are owned by directory entries, and aggregate groups use a directory wherever their list covered it, so a new shared module no longer needs a manifest edit. Small groups keep explicit lists, now checked as a snapshot: the check fails on a reached module that is not declared and on a declared one no longer reached, and prints both per group. --write keeps directory entries as they are. The manifest goes from 4,737 entries to 3,494; src/i18n accounts for 87 of them instead of 666.
This commit is contained in:
1 parent
a90407d547
commit
27939a2d84
5 files changed
+372
-1478
No files matched your search
+14
-5
@@ -55,15 +55,24 @@ See [application construction](APPLICATION.md) and the
|
||||
2. `scripts/check-package-boundaries.mjs` builds every declared export without
|
||||
app Vite configuration or environment files. `scripts/package-boundaries.json`
|
||||
assigns each export exactly once and lists its owned modules and external
|
||||
dependencies; a module entry ending in `/` (such as `src/geo/`) owns every
|
||||
file under that directory. Unused imports still count. Node exports have only a `node`
|
||||
dependencies. Shared foundations use directory entries: `src/i18n/`,
|
||||
`src/geo/` and `src/globe/`. An entry ending in `/` owns the files below it;
|
||||
`src/globe/` excludes `cesium/`, `navara/` and `testing/`, which need separate
|
||||
ownership. The import-direction gate still enforces engine rules.
|
||||
Aggregate groups use directories where their module lists cover the directory;
|
||||
small groups retain explicit snapshots outside the shared foundations.
|
||||
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.
|
||||
entries in order and appending newly reached paths in sorted order. Directory
|
||||
entries stay directories while they own reached modules; they never expand
|
||||
into file lists. Without `--write`, the check rejects both undeclared reached
|
||||
modules and declarations no longer reached, printing additions and removals
|
||||
for each changed group. A directory becomes stale only when no reached module
|
||||
belongs to it. 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,
|
||||
|
||||
@@ -6,7 +6,7 @@ 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. */
|
||||
/** Build each browser or Node export group and check its module snapshot. */
|
||||
export async function checkPackageBoundaries(root) {
|
||||
return buildPackageBoundaries(root, false);
|
||||
}
|
||||
@@ -36,6 +36,7 @@ async function buildPackageBoundaries(root, write) {
|
||||
);
|
||||
}
|
||||
const reports = [];
|
||||
const differences = [];
|
||||
for (const [name, group] of Object.entries(groups)) {
|
||||
const node = group.runtime === 'node';
|
||||
if (group.runtime && !['browser', 'node'].includes(group.runtime)) {
|
||||
@@ -48,12 +49,18 @@ async function buildPackageBoundaries(root, write) {
|
||||
) {
|
||||
throw new Error(`Invalid package boundary: ${name}`);
|
||||
}
|
||||
// A module entry ending in "/" owns every file under that directory.
|
||||
// The shared globe directory excludes engines and their test support.
|
||||
const allowed = new Set();
|
||||
const allowedDirectories = [];
|
||||
const modulePaths = new Map();
|
||||
const globeDirectory = `${normalizePath(path.join(root, 'src/globe'))}/`;
|
||||
const ownsDirectory = (id, directory) =>
|
||||
id.startsWith(directory) &&
|
||||
(directory !== globeDirectory ||
|
||||
!/^(?:cesium|navara|testing)\//.test(id.slice(directory.length)));
|
||||
const owns = (id) =>
|
||||
allowed.has(id) || allowedDirectories.some((dir) => id.startsWith(dir));
|
||||
allowed.has(id) ||
|
||||
allowedDirectories.some((directory) => ownsDirectory(id, directory));
|
||||
for (const module of group.modules) {
|
||||
if (
|
||||
typeof module !== 'string' ||
|
||||
@@ -67,7 +74,7 @@ async function buildPackageBoundaries(root, write) {
|
||||
try {
|
||||
resolved = await realpath(path.join(root, module));
|
||||
} catch (error) {
|
||||
if (write && error.code === 'ENOENT') continue;
|
||||
if (error.code === 'ENOENT') continue;
|
||||
throw error;
|
||||
}
|
||||
const relative = path.relative(root, resolved);
|
||||
@@ -106,11 +113,7 @@ async function buildPackageBoundaries(root, write) {
|
||||
) {
|
||||
throw new Error(`Node export must have only a node condition: ${key}`);
|
||||
}
|
||||
if (
|
||||
typeof target !== 'string' ||
|
||||
!target.startsWith('./') ||
|
||||
(!write && !owns(normalizePath(path.resolve(root, target))))
|
||||
) {
|
||||
if (typeof target !== 'string' || !target.startsWith('./')) {
|
||||
throw new Error(`Export must point to an owned module: ${key}`);
|
||||
}
|
||||
return path.resolve(root, target);
|
||||
@@ -136,11 +139,6 @@ async function buildPackageBoundaries(root, write) {
|
||||
// template blocks belong to the component file itself.
|
||||
if (info.id.startsWith('\0')) return;
|
||||
const id = info.id.split('?')[0];
|
||||
if (!write && !owns(id)) {
|
||||
throw new Error(
|
||||
`Package boundary ${name} imports an unowned module: ${path.relative(root, id)}`,
|
||||
);
|
||||
}
|
||||
seen.add(id);
|
||||
},
|
||||
},
|
||||
@@ -165,38 +163,50 @@ async function buildPackageBoundaries(root, write) {
|
||||
},
|
||||
},
|
||||
});
|
||||
const reached = [...seen];
|
||||
const kept = group.modules.filter((module) => {
|
||||
const resolved = modulePaths.get(module);
|
||||
return module.endsWith('/')
|
||||
? reached.some((id) => ownsDirectory(id, `${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();
|
||||
if (added.length || removed.length)
|
||||
differences.push({ name, added, removed });
|
||||
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) {
|
||||
if (!write && differences.length) {
|
||||
throw new Error(
|
||||
[
|
||||
'Package boundary snapshots differ. Run node scripts/check-package-boundaries.mjs --write and review the diff.',
|
||||
...differences.flatMap(({ name, added, removed }) => [
|
||||
`${name}: ${added.length} added, ${removed.length} removed.`,
|
||||
...added.map((module) => ` + ${module}`),
|
||||
...removed.map((module) => ` - ${module}`),
|
||||
]),
|
||||
].join('\n'),
|
||||
);
|
||||
}
|
||||
if (write && differences.length) {
|
||||
const file = path.join(root, 'scripts/package-boundaries.json');
|
||||
const formatted = await prettier.format(JSON.stringify(manifest), {
|
||||
...(await prettier.resolveConfig(file)),
|
||||
@@ -204,7 +214,7 @@ async function buildPackageBoundaries(root, write) {
|
||||
});
|
||||
await writeFile(file, formatted);
|
||||
}
|
||||
return reports;
|
||||
return write ? differences : reports;
|
||||
}
|
||||
|
||||
const invoked = process.argv[1]
|
||||
|
||||
+173
-1416
File diff suppressed because it is too large.
Load diff
@@ -238,7 +238,12 @@ test('the draw modules are registered for formatting and boundary checks', () =>
|
||||
const boundaries = JSON.parse(read('scripts/package-boundaries.json'));
|
||||
const owners = (file) =>
|
||||
Object.entries(boundaries)
|
||||
.filter(([, group]) => group.modules?.includes(file))
|
||||
.filter(([, group]) =>
|
||||
// An entry ending in '/' owns every module in that directory.
|
||||
group.modules?.some((entry) =>
|
||||
entry.endsWith('/') ? file.startsWith(entry) : entry === file,
|
||||
),
|
||||
)
|
||||
.map(([name]) => name);
|
||||
for (const file of [
|
||||
'src/annotations/drawMode.js',
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { execFileSync, spawnSync } from 'node:child_process';
|
||||
import {
|
||||
copyFile,
|
||||
mkdtemp,
|
||||
@@ -23,7 +23,9 @@ async function fixture(
|
||||
entry = 'export const value = 1;',
|
||||
extraExports = {},
|
||||
) {
|
||||
const root = await mkdtemp(path.join(repoRoot, 'tmp_package_boundaries_'));
|
||||
const root = await mkdtemp(
|
||||
path.join(repoRoot, 'tmp_e11_package_boundaries_'),
|
||||
);
|
||||
t.after(() => rm(root, { recursive: true, force: true }));
|
||||
await mkdir(path.join(root, 'scripts'));
|
||||
await writeFile(
|
||||
@@ -69,7 +71,7 @@ test('even an unused import of application code violates ownership', async (t) =
|
||||
"import { app } from './startup.js'; export const value = 1;",
|
||||
);
|
||||
await writeFile(path.join(root, 'startup.js'), 'export const app = 2;');
|
||||
await assert.rejects(checkPackageBoundaries(root), /unowned module.*startup/);
|
||||
await assert.rejects(checkPackageBoundaries(root), /\+ startup\.js/);
|
||||
});
|
||||
|
||||
test('Node builtins cannot enter a browser export', async (t) => {
|
||||
@@ -77,7 +79,10 @@ test('Node builtins cannot enter a browser export', async (t) => {
|
||||
t,
|
||||
"import fs from 'node:fs'; export const value = fs;",
|
||||
);
|
||||
await assert.rejects(checkPackageBoundaries(root), /unowned module|browser/);
|
||||
await assert.rejects(
|
||||
checkPackageBoundaries(root),
|
||||
/browser|escapes repository/,
|
||||
);
|
||||
});
|
||||
|
||||
test('dynamic imports obey the same component ownership rule', async (t) => {
|
||||
@@ -86,7 +91,7 @@ test('dynamic imports obey the same component ownership rule', async (t) => {
|
||||
"export const load = () => import('./startup.js');",
|
||||
);
|
||||
await writeFile(path.join(root, 'startup.js'), 'export const app = 2;');
|
||||
await assert.rejects(checkPackageBoundaries(root), /unowned module.*startup/);
|
||||
await assert.rejects(checkPackageBoundaries(root), /\+ startup\.js/);
|
||||
});
|
||||
|
||||
test('a Node build export must be explicitly classified and scoped', async (t) => {
|
||||
@@ -118,7 +123,7 @@ test('a Node build export must be explicitly classified and scoped', async (t) =
|
||||
"import './startup.js'; export const value = 1;",
|
||||
);
|
||||
await writeFile(path.join(root, 'startup.js'), 'export const app = 2;');
|
||||
await assert.rejects(checkPackageBoundaries(root), /unowned module.*startup/);
|
||||
await assert.rejects(checkPackageBoundaries(root), /\+ startup\.js/);
|
||||
});
|
||||
|
||||
test('a lifecycle owner cannot acquire a layer panel even through an unused import', async (t) => {
|
||||
@@ -137,7 +142,7 @@ test('a lifecycle owner cannot acquire a layer panel even through an unused impo
|
||||
},
|
||||
}),
|
||||
);
|
||||
await assert.rejects(checkPackageBoundaries(root), /unowned module.*panel/);
|
||||
await assert.rejects(checkPackageBoundaries(root), /\+ panel\.js/);
|
||||
});
|
||||
|
||||
test('a module entry ending in a slash owns every file under that directory', async (t) => {
|
||||
@@ -151,6 +156,10 @@ test('a module entry ending in a slash owns every file under that directory', as
|
||||
"export { b as a } from './nested/b.js';",
|
||||
);
|
||||
await writeFile(path.join(root, 'shared/nested/b.js'), 'export const b = 1;');
|
||||
await writeFile(
|
||||
path.join(root, 'shared/unreached.js'),
|
||||
'export const c = 2;',
|
||||
);
|
||||
await writeFile(
|
||||
path.join(root, 'scripts/package-boundaries.json'),
|
||||
JSON.stringify({
|
||||
@@ -170,10 +179,7 @@ test('a module entry ending in a slash owns every file under that directory', as
|
||||
"export { c as b } from '../../shared.js';",
|
||||
);
|
||||
await writeFile(path.join(root, 'shared.js'), 'export const c = 1;');
|
||||
await assert.rejects(
|
||||
checkPackageBoundaries(root),
|
||||
/unowned module.*shared\.js/,
|
||||
);
|
||||
await assert.rejects(checkPackageBoundaries(root), /\+ shared\.js/);
|
||||
});
|
||||
|
||||
test('regeneration adds unowned modules, including unused and dynamic imports', async (t) => {
|
||||
@@ -199,16 +205,17 @@ test('regeneration adds unowned modules, including unused and dynamic imports',
|
||||
]);
|
||||
});
|
||||
|
||||
test('regeneration removes unreached files without making the check reject stale entries', async (t) => {
|
||||
test('the check rejects unreached files and regeneration removes them', 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 },
|
||||
]);
|
||||
await assert.rejects(
|
||||
checkPackageBoundaries(root),
|
||||
/feature: 0 added, 1 removed\.\n - stale\.js/,
|
||||
);
|
||||
assert.deepEqual(await writePackageBoundaries(root), [
|
||||
{ name: 'feature', added: [], removed: ['stale.js'] },
|
||||
]);
|
||||
@@ -223,6 +230,10 @@ test('regeneration removes deleted entries and adds an unowned export entry', as
|
||||
const manifest = JSON.parse(await readFile(file, 'utf8'));
|
||||
manifest.feature.modules = ['deleted.js', 'deleted/'];
|
||||
await writeFile(file, JSON.stringify(manifest));
|
||||
await assert.rejects(
|
||||
checkPackageBoundaries(root),
|
||||
/feature: 1 added, 2 removed\.\n \+ entry\.js\n - deleted\.js\n - deleted\//,
|
||||
);
|
||||
assert.deepEqual(await writePackageBoundaries(root), [
|
||||
{
|
||||
name: 'feature',
|
||||
@@ -262,6 +273,10 @@ test('regeneration keeps a directory only while it owns a reached module', async
|
||||
path.join(root, 'entry.js'),
|
||||
"export { other } from './shared.js';",
|
||||
);
|
||||
await assert.rejects(
|
||||
checkPackageBoundaries(root),
|
||||
/feature: 0 added, 1 removed\.\n - shared\//,
|
||||
);
|
||||
assert.deepEqual(await writePackageBoundaries(root), [
|
||||
{ name: 'feature', added: [], removed: ['shared/'] },
|
||||
]);
|
||||
@@ -271,6 +286,90 @@ test('regeneration keeps a directory only while it owns a reached module', async
|
||||
]);
|
||||
});
|
||||
|
||||
test('shared globe ownership excludes engine and testing directories', async (t) => {
|
||||
const root = await fixture(
|
||||
t,
|
||||
"export { value } from './src/globe/shared/value.js';",
|
||||
);
|
||||
await mkdir(path.join(root, 'src/globe/shared'), { recursive: true });
|
||||
await writeFile(
|
||||
path.join(root, 'src/globe/shared/value.js'),
|
||||
'export const value = 1;',
|
||||
);
|
||||
const file = path.join(root, 'scripts/package-boundaries.json');
|
||||
const manifest = JSON.parse(await readFile(file, 'utf8'));
|
||||
manifest.feature.modules.push('src/globe/');
|
||||
await writeFile(file, JSON.stringify(manifest));
|
||||
assert.deepEqual(await checkPackageBoundaries(root), [
|
||||
{ name: 'feature', exports: 1, modules: 2 },
|
||||
]);
|
||||
assert.deepEqual(await writePackageBoundaries(root), []);
|
||||
for (const directory of ['cesium', 'navara', 'testing']) {
|
||||
await mkdir(path.join(root, 'src/globe', directory));
|
||||
const module = `src/globe/${directory}/value.js`;
|
||||
await writeFile(path.join(root, module), 'export const value = 2;');
|
||||
await writeFile(
|
||||
path.join(root, 'entry.js'),
|
||||
`export { value } from './src/globe/shared/value.js'; import './${module}';`,
|
||||
);
|
||||
await assert.rejects(checkPackageBoundaries(root), (error) => {
|
||||
assert.ok(
|
||||
error.message.includes(`feature: 1 added, 0 removed.\n + ${module}`),
|
||||
);
|
||||
return true;
|
||||
});
|
||||
assert.deepEqual(await writePackageBoundaries(root), [
|
||||
{ name: 'feature', added: [module], removed: [] },
|
||||
]);
|
||||
assert.deepEqual(JSON.parse(await readFile(file, 'utf8')).feature.modules, [
|
||||
'entry.js',
|
||||
'src/globe/',
|
||||
module,
|
||||
]);
|
||||
assert.deepEqual(await checkPackageBoundaries(root), [
|
||||
{ name: 'feature', exports: 1, modules: 3 },
|
||||
]);
|
||||
await writeFile(file, JSON.stringify(manifest));
|
||||
}
|
||||
});
|
||||
|
||||
test('the check reports additions and removals for every changed group', async (t) => {
|
||||
const root = await fixture(t, "export { value } from './value.js';", {
|
||||
'./other': './other.js',
|
||||
});
|
||||
for (const name of ['value.js', 'stale.js', 'other.js', 'other-stale.js']) {
|
||||
await writeFile(path.join(root, name), 'export const value = 1;');
|
||||
}
|
||||
const file = path.join(root, 'scripts/package-boundaries.json');
|
||||
const manifest = JSON.parse(await readFile(file, 'utf8'));
|
||||
manifest.feature.modules.push('stale.js');
|
||||
manifest.other = {
|
||||
exports: ['./other'],
|
||||
modules: ['other.js', 'other-stale.js'],
|
||||
external: [],
|
||||
};
|
||||
const original = JSON.stringify(manifest);
|
||||
await writeFile(file, original);
|
||||
await assert.rejects(checkPackageBoundaries(root), (error) => {
|
||||
assert.equal(
|
||||
error.message,
|
||||
'Package boundary snapshots differ. Run node scripts/check-package-boundaries.mjs --write and review the diff.\n' +
|
||||
'feature: 1 added, 1 removed.\n + value.js\n - stale.js\n' +
|
||||
'other: 0 added, 1 removed.\n - other-stale.js',
|
||||
);
|
||||
return true;
|
||||
});
|
||||
assert.equal(await readFile(file, 'utf8'), original);
|
||||
assert.deepEqual(await writePackageBoundaries(root), [
|
||||
{ name: 'feature', added: ['value.js'], removed: ['stale.js'] },
|
||||
{ name: 'other', added: [], removed: ['other-stale.js'] },
|
||||
]);
|
||||
assert.deepEqual(await checkPackageBoundaries(root), [
|
||||
{ name: 'feature', exports: 1, modules: 2 },
|
||||
{ name: 'other', exports: 1, modules: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
test('regeneration preserves kept entry order and all other manifest fields', async (t) => {
|
||||
const root = await fixture(
|
||||
t,
|
||||
@@ -344,6 +443,20 @@ test('the CLI reports changes and a second --write leaves the manifest untouched
|
||||
env: { ...process.env, GOOGLE_MAPS_API_KEY: '', CESIUM_ION_TOKEN: '' },
|
||||
},
|
||||
);
|
||||
const checked = spawnSync(
|
||||
process.execPath,
|
||||
['scripts/check-package-boundaries.mjs'],
|
||||
{
|
||||
cwd: root,
|
||||
encoding: 'utf8',
|
||||
env: { ...process.env, GOOGLE_MAPS_API_KEY: '', CESIUM_ION_TOKEN: '' },
|
||||
},
|
||||
);
|
||||
assert.equal(checked.status, 1);
|
||||
assert.match(
|
||||
checked.stderr,
|
||||
/feature: 1 added, 1 removed\.\n \+ value\.js\n - stale\.js/,
|
||||
);
|
||||
assert.equal(
|
||||
run(['--write']),
|
||||
'Updated feature: 1 added, 1 removed.\n + value.js\n - stale.js\n',
|
||||
|
||||
Reference in new issue
Block a user