diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6a322ea9..167ea65a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,12 @@ jobs: - name: Run setup policy checks run: npm run doctor -- --json + - name: Check adopted formatting + run: npm run format:check + + - name: Check package boundaries + run: npm run check:boundaries + - name: Run unit tests run: npm test @@ -70,6 +76,12 @@ jobs: - name: Run the Pinokio install path run: node scripts/pinokio-install.mjs + - name: Check adopted formatting + run: npm run format:check + + - name: Check package boundaries + run: npm run check:boundaries + - name: Run focused onboarding tests run: node --test src/setupDoctor.test.mjs src/pinokioEnvironment.test.mjs src/pinokioPreflight.test.mjs src/pinokioLauncherContract.test.mjs src/mapStartup.test.mjs src/hudSummaryResponse.test.mjs diff --git a/.prettierignore b/.prettierignore index fa207e9f0..1aaad8a03 100644 --- a/.prettierignore +++ b/.prettierignore @@ -11,4 +11,12 @@ pinokio/.installed src/data/local_data/ *.geojson *.geojsonl -*.html + +# Local configuration, generated assets and browser evidence +.env +.env.* +!.env.example +node_modules +.vite +.vite-* +*.min.js diff --git a/.prettierrc.json b/.prettierrc.json index 217337eae..b1e6b2baf 100644 --- a/.prettierrc.json +++ b/.prettierrc.json @@ -1,4 +1,6 @@ { "singleQuote": true, - "tabWidth": 2 + "tabWidth": 2, + "semi": true, + "endOfLine": "lf" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f3415c2a..00d24586c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md ## [Unreleased] +### Development + +- Adopt Prettier tooling contributed by RohanDaCoder (#227), with an explicit + file scope, pinned formatter and Linux/Windows CI checks. Format the reusable + infrastructure modules and their consumer tests. Package boundary checks keep + those exports separate from app startup and local Node services. + ### Fixed - Separate optional Google server credentials for Places and Street View from diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6760dfdf1..62da95f5c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,6 +51,22 @@ The highest-leverage places to jump in: - Match the surrounding code — comment density, naming, and idiom. - Prefer small, reviewable commits. Conventional-commit-style prefixes (`feat:`, `fix:`, `perf:`, `docs:`) are appreciated but not required. +## Formatting and reusable components + +Run `npm run format` before submitting changes to adopted modules, then +`npm run format:check` and `npm run check:boundaries`. Formatting uses the +explicit file list in `scripts/format-scope.json`; add newly extracted modules +and tests there in a separate mechanical commit. Keep unadopted files consistent +with their surrounding style. CI checks all adopted files and package boundaries +on Linux and Windows. + +Reusable package exports own their state and receive application operations +through explicit callbacks. They must not import the standalone bootstrap or +local Node services. Update `scripts/package-boundaries.json` and consumer tests +when adding an export or expanding a component. See +[formatting and component boundaries](docs/CODE-BOUNDARIES.md) for the current +ownership and adoption process. + ## Pull requests 1. Branch off `main`. diff --git a/docs/CODE-BOUNDARIES.md b/docs/CODE-BOUNDARIES.md new file mode 100644 index 000000000..2d4df7ad5 --- /dev/null +++ b/docs/CODE-BOUNDARIES.md @@ -0,0 +1,50 @@ +# Formatting and component boundaries + +Run `npm run format` to format the files in `scripts/format-scope.json` and +`npm run format:check` to check that same list without writing. CI checks the +entire adopted list on Linux and Windows. Prettier is pinned in the development +dependencies; use the installed version so local and CI output agree. The shared +configuration specifies two spaces, single quotes, semicolons and LF endings. + +Add new reusable modules and their tests to the list as they are extracted. +Keep mechanical formatting in its own commit after behavior is stable. Existing +source-text regression assertions still apply; investigate failures and preserve +their behavioral coverage when a move or line wrap changes a tested shape. +Files outside the list retain their surrounding style until deliberately adopted. +Generated output, local configuration, browser evidence and bundled datasets are +excluded. The formatter validates every entry before writing any file. + +## Current component ownership + +| Surface | Owns | Receives from its caller | +| -------------------------------------- | ---------------------------------------------------------------------- | --------------------------------------------------- | +| `gods-eye-view/infrastructure` | Datacenter/dam definitions and fresh layer construction | Context, overlay and render operations | +| `gods-eye-view/infrastructure/geojson` | Data loading, Cesium entities, selection handling and resource cleanup | A viewer and those same operations | +| `gods-eye-view/infrastructure/lod` | Pure visibility budgets and selection policy | Position/visibility records and camera measurements | +| `src/data/localGeojson.js` | Standalone compatibility wiring | The application's existing shared services | +| `src/main.js` and `vite.config.js` | Standalone startup and local Node services | Local configuration | + +The three scoped package exports are browser source modules. Use their documented +exports instead of importing standalone startup or reaching into internal files. +The application owns the viewer, context store, overlay host and render scheduler; +layers use the supplied callbacks. See [the infrastructure contract](INFRASTRUCTURE-LAYERS.md). + +`npm run check:boundaries` builds every declared package export, with app Vite +configuration disabled. `scripts/package-boundaries.json` lists each export's +component, owned modules and external runtime dependencies. A new export must be +classified. Imports outside the declared modules fail, including unused and +literal dynamic imports. Cesium stays external so the consuming application +supplies the same compatible instance as its viewer. Existing consumer tests +also check import-time inactivity and asset URLs under a non-root base. + +These checks cover the declared exports, not every import in the application. +They check build-time imports, not arbitrary runtime-generated module URLs. +Keep runtime module discovery out of these exports. When extracting another +component, add its ownership and consumer tests together. Node services must use +separate entry points and their own checks when they become reusable; importing +them into a browser component is not supported. No Node service is exported yet. + +This setup establishes boundaries for the existing infrastructure component. +Application construction, UI panels and individual source adapters remain future +extractions; they should become smaller modules with explicit lifecycle owners +as their callers migrate. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 4485b1026..cea6f4a53 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1,5 +1,14 @@ # God's Eye View Current State +## Scoped formatting and package checks + +`npm run format` and `npm run format:check` operate on the explicit adopted-file +list. `npm run check:boundaries` checks the browser dependency graph of all +current package exports; infrastructure owns its three implementation modules +and takes Cesium from the consumer. CI runs both checks on Linux and Windows. +The standalone app, layer behavior and public export paths remain unchanged. +See [component ownership and adoption](CODE-BOUNDARIES.md). + ## Google browser and server keys Local Places nearby/text search and the CCTV Street View fallback prefer diff --git a/package-lock.json b/package-lock.json index f835ac31d..c24590e4a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,9 +17,7 @@ "satellite.js": "^6.0.2" }, "devDependencies": { - "prettier": "^3.9.6", - "puppeteer": "^24.37.5", - "sharp": "^0.34.5", + "prettier": "3.9.6", "puppeteer": "^25.10.0", "sharp": "^0.35.4", "vite": "^6.0.0", @@ -2239,16 +2237,6 @@ "url": "https://github.com/prettier/prettier?sponsor=1" } }, - "node_modules/progress": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/progress/-/progress-2.0.3.tgz", - "integrity": "sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.4.0" - } - }, "node_modules/protobufjs": { "version": "8.8.0", "resolved": "https://registry.npmjs.org/protobufjs/-/protobufjs-8.8.0.tgz", diff --git a/package.json b/package.json index b32405ac5..dcf0b4a77 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "gods-eye-view", "private": true, "version": "0.1.1", - "description": "A real-time intelligence console for planet Earth — photorealistic 3D globe, live aircraft/ships/satellites/earthquakes/CCTV, and hands-free voice control. Runs in a browser.", + "description": "A real-time intelligence console for planet Earth \u2014 photorealistic 3D globe, live aircraft/ships/satellites/earthquakes/CCTV, and hands-free voice control. Runs in a browser.", "type": "module", "license": "MIT", "author": "Bilawal Sidhu", @@ -33,8 +33,8 @@ ], "scripts": { "doctor": "node scripts/setup-doctor.mjs", - "format": "prettier --write .", - "format:check": "prettier --check .", + "format": "node scripts/format.mjs --write", + "format:check": "node scripts/format.mjs --check", "dev": "vite", "dev:secure": "./scripts/dev-secure.sh", "opensky:import": "./scripts/opensky-import-client.sh", @@ -42,7 +42,8 @@ "preview": "vite preview", "test": "node scripts/run-unit-tests.mjs", "test:track": "node scripts/track-regression.mjs", - "qa:map-source-tray": "node scripts/qa-map-source-tray.mjs" + "qa:map-source-tray": "node scripts/qa-map-source-tray.mjs", + "check:boundaries": "node scripts/check-package-boundaries.mjs" }, "dependencies": { "@mapbox/vector-tile": "^3.0.0", @@ -53,9 +54,7 @@ "satellite.js": "^6.0.2" }, "devDependencies": { - "prettier": "^3.9.6", - "puppeteer": "^24.37.5", - "sharp": "^0.34.5", + "prettier": "3.9.6", "puppeteer": "^25.10.0", "sharp": "^0.35.4", "vite": "^6.0.0", diff --git a/scripts/check-package-boundaries.mjs b/scripts/check-package-boundaries.mjs new file mode 100644 index 000000000..50abe15c2 --- /dev/null +++ b/scripts/check-package-boundaries.mjs @@ -0,0 +1,131 @@ +import { readFile, realpath } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { build, normalizePath } from 'vite'; + +/** Build each browser export group and reject imports outside its declared ownership. */ +export async function checkPackageBoundaries(root) { + root = await realpath(root); + const readJson = async (name) => + JSON.parse(await readFile(path.join(root, name), 'utf8')); + const pkg = await readJson('package.json'); + const groups = await readJson('scripts/package-boundaries.json'); + const declaredExports = Object.keys(pkg.exports || {}).sort(); + const classifiedExports = Object.values(groups) + .flatMap((group) => group.exports) + .sort(); + if ( + !declaredExports.length || + JSON.stringify(declaredExports) !== JSON.stringify(classifiedExports) + ) { + throw new Error( + 'Every package export must belong to exactly one boundary group', + ); + } + const reports = []; + for (const [name, group] of Object.entries(groups)) { + if ( + !Array.isArray(group.modules) || + !group.modules.length || + !Array.isArray(group.external) + ) { + throw new Error(`Invalid package boundary: ${name}`); + } + const allowed = new Set(); + for (const module of group.modules) { + if ( + typeof module !== 'string' || + path.isAbsolute(module) || + module.includes('\\') || + module.split('/').includes('..') + ) { + throw new Error(`Boundary modules must be repository paths: ${name}`); + } + const resolved = await realpath(path.join(root, module)); + const relative = path.relative(root, resolved); + if ( + relative === '..' || + relative.startsWith(`..${path.sep}`) || + path.isAbsolute(relative) + ) { + throw new Error(`Boundary module escapes repository: ${name}`); + } + allowed.add(normalizePath(resolved)); + } + for (const external of group.external) { + if ( + !Object.hasOwn(pkg.dependencies || {}, external) && + !Object.hasOwn(pkg.peerDependencies || {}, external) + ) { + throw new Error( + `Boundary external must be a declared runtime dependency: ${external}`, + ); + } + } + const input = group.exports.map((key) => { + const target = pkg.exports[key]; + if ( + typeof target !== 'string' || + !target.startsWith('./') || + !allowed.has(normalizePath(path.resolve(root, target))) + ) { + throw new Error(`Export must point to an owned module: ${key}`); + } + return path.resolve(root, target); + }); + const seen = new Set(); + await build({ + root, + configFile: false, + envFile: false, + publicDir: false, + logLevel: 'silent', + plugins: [ + { + name: 'check-package-ownership', + moduleParsed(info) { + if (!allowed.has(info.id)) { + throw new Error( + `Package boundary ${name} imports an unowned module: ${path.relative(root, info.id)}`, + ); + } + seen.add(info.id); + }, + }, + ], + build: { + lib: { entry: input, formats: ['es'] }, + write: false, + minify: false, + assetsInlineLimit: 0, + rollupOptions: { + input, + external: group.external, + // Unused imports must still obey ownership; tree shaking is not a boundary. + treeshake: false, + preserveEntrySignatures: 'strict', + }, + }, + }); + reports.push({ name, exports: input.length, modules: seen.size }); + } + return reports; +} + +const invoked = process.argv[1] + ? pathToFileURL(path.resolve(process.argv[1])).href + : ''; +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.`, + ); + } catch (error) { + console.error(error.message); + process.exitCode = 1; + } +} diff --git a/scripts/format-scope.json b/scripts/format-scope.json new file mode 100644 index 000000000..647fffacf --- /dev/null +++ b/scripts/format-scope.json @@ -0,0 +1,15 @@ +[ + ".prettierrc.json", + "package.json", + "scripts/format-scope.json", + "scripts/package-boundaries.json", + "scripts/format.mjs", + "scripts/check-package-boundaries.mjs", + "src/tooling/format.test.mjs", + "src/tooling/packageBoundaries.test.mjs", + "src/data/infrastructure.js", + "src/data/infrastructure.test.mjs", + "src/data/localGeojsonCore.js", + "src/data/localGeojsonLod.js", + "docs/CODE-BOUNDARIES.md" +] diff --git a/scripts/format.mjs b/scripts/format.mjs new file mode 100644 index 000000000..b74dfbdcd --- /dev/null +++ b/scripts/format.mjs @@ -0,0 +1,101 @@ +import { readFile, writeFile, realpath, stat } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import * as prettier from 'prettier'; + +/** Check or format the explicitly adopted files, without scanning other source. */ +export async function formatAdoptedFiles(root, mode) { + if (!['--check', '--write'].includes(mode)) { + throw new Error('Usage: node scripts/format.mjs --check|--write'); + } + root = await realpath(root); + const scope = JSON.parse( + await readFile(path.join(root, 'scripts/format-scope.json'), 'utf8'), + ); + if ( + !Array.isArray(scope) || + !scope.length || + new Set(scope).size !== scope.length + ) { + throw new Error( + 'Formatting scope must be a nonempty list of unique file paths', + ); + } + + // Validate the whole scope before any write, including resolved symlink paths. + const files = []; + for (const name of scope) { + if ( + typeof name !== 'string' || + !name || + name.includes('\\') || + path.isAbsolute(name) || + name.split('/').includes('..') + ) { + throw new Error( + 'Formatting scope entries must be paths inside the repository', + ); + } + const file = path.join(root, name); + const relative = path.relative(root, await realpath(file)); + if ( + relative === '..' || + relative.startsWith(`..${path.sep}`) || + path.isAbsolute(relative) || + !(await stat(file)).isFile() + ) { + throw new Error( + `Formatting scope must contain repository files: ${name}`, + ); + } + const info = await prettier.getFileInfo(file, { + ignorePath: path.join(root, '.prettierignore'), + }); + if (info.ignored || !info.inferredParser) { + throw new Error( + `Formatting scope contains an ignored or unsupported file: ${name}`, + ); + } + const options = { ...(await prettier.resolveConfig(file)), filepath: file }; + const source = await readFile(file, 'utf8'); + files.push({ + name, + file, + source, + formatted: await prettier.format(source, options), + }); + } + + const changed = files.filter(({ source, formatted }) => source !== formatted); + if (mode === '--write') { + for (const { file, formatted } of changed) await writeFile(file, formatted); + } + return { count: files.length, changed: changed.map(({ name }) => name) }; +} + +const invoked = process.argv[1] + ? pathToFileURL(path.resolve(process.argv[1])).href + : ''; +if (import.meta.url === invoked) { + try { + if (process.argv.length !== 3) + throw new Error('Usage: node scripts/format.mjs --check|--write'); + const mode = process.argv[2]; + const result = await formatAdoptedFiles( + fileURLToPath(new URL('../', import.meta.url)), + mode, + ); + if (mode === '--check' && result.changed.length) { + for (const name of result.changed) + console.error(`Needs formatting: ${name}`); + process.exitCode = 1; + } else { + console.log( + `${mode === '--write' ? 'Formatted' : 'Checked'} ${result.count} adopted files.`, + ); + } + } catch (error) { + console.error(error.message); + process.exitCode = 1; + } +} diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json new file mode 100644 index 000000000..20fa4547b --- /dev/null +++ b/scripts/package-boundaries.json @@ -0,0 +1,15 @@ +{ + "infrastructure": { + "exports": [ + "./infrastructure", + "./infrastructure/geojson", + "./infrastructure/lod" + ], + "modules": [ + "src/data/infrastructure.js", + "src/data/localGeojsonCore.js", + "src/data/localGeojsonLod.js" + ], + "external": ["cesium"] + } +} diff --git a/src/tooling/format.test.mjs b/src/tooling/format.test.mjs new file mode 100644 index 000000000..f0ab4405c --- /dev/null +++ b/src/tooling/format.test.mjs @@ -0,0 +1,95 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + mkdtemp, + mkdir, + writeFile, + readFile, + rm, + symlink, +} from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { formatAdoptedFiles } from '../../scripts/format.mjs'; + +async function fixture(t, scope = ['adopted.js']) { + const root = await mkdtemp(path.join(tmpdir(), 'gev-format-')); + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(path.join(root, 'scripts')); + await writeFile( + path.join(root, 'scripts/format-scope.json'), + JSON.stringify(scope), + ); + await writeFile( + path.join(root, '.prettierrc.json'), + JSON.stringify({ + singleQuote: true, + semi: true, + tabWidth: 2, + endOfLine: 'lf', + }), + ); + await writeFile(path.join(root, '.prettierignore'), 'ignored.js\n'); + await writeFile(path.join(root, 'adopted.js'), 'const value="yes"\r\n'); + return root; +} + +test('format check is read-only; write only changes adopted files and is repeatable', async (t) => { + const root = await fixture(t); + const outside = 'const untouched="yes"'; + await writeFile(path.join(root, 'legacy.js'), outside); + assert.deepEqual((await formatAdoptedFiles(root, '--check')).changed, [ + 'adopted.js', + ]); + assert.equal( + await readFile(path.join(root, 'adopted.js'), 'utf8'), + 'const value="yes"\r\n', + ); + await formatAdoptedFiles(root, '--write'); + assert.equal( + await readFile(path.join(root, 'adopted.js'), 'utf8'), + "const value = 'yes';\n", + ); + assert.equal(await readFile(path.join(root, 'legacy.js'), 'utf8'), outside); + assert.deepEqual((await formatAdoptedFiles(root, '--check')).changed, []); + assert.deepEqual((await formatAdoptedFiles(root, '--write')).changed, []); +}); + +test('invalid, ignored and missing scope entries fail before any file is written', async (t) => { + for (const scope of [ + [], + ['adopted.js', 'adopted.js'], + ['adopted.js', '../escape.js'], + ['adopted.js', 'ignored.js'], + ['adopted.js', 'missing.js'], + ]) { + const root = await fixture(t, scope); + await writeFile(path.join(root, 'ignored.js'), 'const ignored=1'); + await assert.rejects(formatAdoptedFiles(root, '--write')); + assert.equal( + await readFile(path.join(root, 'adopted.js'), 'utf8'), + 'const value="yes"\r\n', + ); + } +}); + +test( + 'formatting rejects a symlink outside the repository', + { skip: process.platform === 'win32' }, + async (t) => { + const root = await fixture(t, ['adopted.js', 'outside.js']); + const other = await fixture(t); + await symlink( + path.join(other, 'adopted.js'), + path.join(root, 'outside.js'), + ); + await assert.rejects( + formatAdoptedFiles(root, '--write'), + /repository files/, + ); + assert.equal( + await readFile(path.join(root, 'adopted.js'), 'utf8'), + 'const value="yes"\r\n', + ); + }, +); diff --git a/src/tooling/packageBoundaries.test.mjs b/src/tooling/packageBoundaries.test.mjs new file mode 100644 index 000000000..18c4146d4 --- /dev/null +++ b/src/tooling/packageBoundaries.test.mjs @@ -0,0 +1,77 @@ +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 path from 'node:path'; +import { checkPackageBoundaries } from '../../scripts/check-package-boundaries.mjs'; + +async function fixture( + t, + entry = 'export const value = 1;', + extraExports = {}, +) { + const root = await mkdtemp(path.join(tmpdir(), 'gev-boundaries-')); + t.after(() => rm(root, { recursive: true, force: true })); + await mkdir(path.join(root, 'scripts')); + await writeFile( + path.join(root, 'package.json'), + JSON.stringify({ + name: 'boundary-fixture', + type: 'module', + exports: { './feature': './entry.js', ...extraExports }, + }), + ); + await writeFile( + path.join(root, 'scripts/package-boundaries.json'), + JSON.stringify({ + feature: { exports: ['./feature'], modules: ['entry.js'], external: [] }, + }), + ); + await writeFile(path.join(root, 'entry.js'), entry); + return root; +} + +test('a declared browser export builds without running app setup', async (t) => { + const root = await fixture(t); + await writeFile( + path.join(root, 'vite.config.js'), + "throw new Error('must not load app config');", + ); + assert.deepEqual(await checkPackageBoundaries(root), [ + { name: 'feature', exports: 1, modules: 1 }, + ]); +}); + +test('an unclassified package export fails the boundary gate', async (t) => { + const root = await fixture(t, undefined, { './another': './another.js' }); + await assert.rejects( + checkPackageBoundaries(root), + /exactly one boundary group/, + ); +}); + +test('even an unused import of application code violates ownership', async (t) => { + const root = await fixture( + 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/); +}); + +test('Node builtins cannot enter a browser export', async (t) => { + const root = await fixture( + t, + "import fs from 'node:fs'; export const value = fs;", + ); + await assert.rejects(checkPackageBoundaries(root), /unowned module|browser/); +}); + +test('dynamic imports obey the same component ownership rule', async (t) => { + const root = await fixture( + 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/); +});