From 9c87fabf883c5dd33768d5fd3d673aadd77e9c6e Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Fri, 2 Oct 2026 20:54:54 -0700 Subject: [PATCH] build: ship each bundled data pack once loadBundledJson's callers passed a literal JSON import as a Node fallback, which made the production build emit a JavaScript copy of every pack beside the JSON the browser fetches. The loader now reads file: URLs itself under Node (through process.getBuiltinModule, synchronously, as promptly as the cached import it replaces), and the callers pass only the URL. On a Node too old to provide getBuiltinModule it says that Node 24.14 or newer is needed. dist/ drops from 54 MB to 42 MB. A test keeps app code from importing a pack as a module. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 + src/data/adminBoundaries.js | 24 +-- src/data/bundledJson.js | 30 +++- src/data/bundledJson.test.mjs | 67 ++++++-- src/data/militaryNames.js | 4 - src/data/naturalEarthRegions.js | 12 +- src/data/neighborhoodPolygons.js | 6 +- .../installations/namesLoading.test.mjs | 162 ++++++++++++------ 8 files changed, 203 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d39007b3a..5db2da074 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## [Unreleased] +- Ship each bundled data pack once. The region, marine, admin-boundary, + county, military-name and neighborhood packs were emitted twice by the + production build, as the JSON the browser fetches and as an unused + JavaScript copy; `dist/` drops from 54 MB to 42 MB. Under Node the loader + reads the JSON file directly (reporting that Node 24.14 or newer is needed + on a runtime too old to do so), and a test keeps app code from importing a + pack as a module. + - Apply the Host check before the app's own routes. Vite installs its Host check after the middleware that plugins add, so the provider and `/api` routes used to answer any Host, including a DNS-rebinding name, in every diff --git a/src/data/adminBoundaries.js b/src/data/adminBoundaries.js index 608325ca9..e9f97cf8f 100644 --- a/src/data/adminBoundaries.js +++ b/src/data/adminBoundaries.js @@ -58,30 +58,18 @@ export function normalizeAdminName(value) { const PACKS = { countries: { url: new URL('./local_data/natural_earth/countries.json', import.meta.url), - importJson: () => - import('./local_data/natural_earth/countries.json', { - with: { type: 'json' }, - }), }, admin1: { url: new URL( './local_data/natural_earth/states_provinces.json', import.meta.url, ), - importJson: () => - import('./local_data/natural_earth/states_provinces.json', { - with: { type: 'json' }, - }), }, counties: { url: new URL( './local_data/us_census_counties/counties.json', import.meta.url, ), - importJson: () => - import('./local_data/us_census_counties/counties.json', { - with: { type: 'json' }, - }), }, }; @@ -297,7 +285,7 @@ function addKey(index, key, entry, tier) { const loadAdmin1 = createRetryableLoader(async () => { packLoads.admin1 += 1; - const pack = await loadBundledJson(PACKS.admin1.url, PACKS.admin1.importJson); + const pack = await loadBundledJson(PACKS.admin1.url); const decimals = pack.meta?.decimals ?? 3; const index = new Map(); for (const feature of pack.features || []) { @@ -323,10 +311,7 @@ const loadAdmin1 = createRetryableLoader(async () => { const loadCounties = createRetryableLoader(async () => { packLoads.counties += 1; - const pack = await loadBundledJson( - PACKS.counties.url, - PACKS.counties.importJson, - ); + const pack = await loadBundledJson(PACKS.counties.url); const decimals = pack.meta?.decimals ?? 4; const byFull = new Map(); const byName = new Map(); @@ -348,10 +333,7 @@ const loadCounties = createRetryableLoader(async () => { const loadCountries = createRetryableLoader(async () => { packLoads.countries += 1; - const pack = await loadBundledJson( - PACKS.countries.url, - PACKS.countries.importJson, - ); + const pack = await loadBundledJson(PACKS.countries.url); const index = new Map(); for (const feature of pack.features || []) { const entry = { diff --git a/src/data/bundledJson.js b/src/data/bundledJson.js index b15656c31..7110a3d95 100644 --- a/src/data/bundledJson.js +++ b/src/data/bundledJson.js @@ -4,24 +4,38 @@ * The browser fetches the file from `url` — a `new URL('./….json', * import.meta.url)` written in the calling module, which Vite serves as-is in * dev and emits as an asset in the production build. Under Node (node:test) - * that URL is a `file:` URL, which `fetch` cannot read, so the caller's - * JSON-attributed import runs instead. + * that URL is a `file:` URL, which `fetch` cannot read, so the file is read + * from disk instead. * - * The attributed import cannot serve the browser: the Vite dev server answers - * it with a JavaScript module, which the browser rejects for a JSON import. + * Nothing here imports the JSON as a module. A JSON-attributed import cannot + * serve the browser (the Vite dev server answers it with a JavaScript module, + * which the browser rejects), and a literal `import('./….json')` makes the + * production build emit a JavaScript copy of every pack that the browser + * never loads. * * @module data/bundledJson */ /** * @param {URL} url - Resolved against the calling module's `import.meta.url`. - * @param {() => Promise<{default: any}>} importJson - The caller's literal - * `import('./….json', { with: { type: 'json' } })`, used only under Node. + * @param {{signal?: AbortSignal}} [options] * @returns {Promise} The parsed JSON. */ -export async function loadBundledJson(url, importJson, { signal } = {}) { - if (url.protocol === 'file:') return (await importJson()).default; +export async function loadBundledJson(url, { signal } = {}) { signal?.throwIfAborted(); + if (url.protocol === 'file:') { + // Node's built-in, reached without an import, so the browser build never + // bundles or rewrites it; only Node, reading `file:` URLs, gets here. The + // read is synchronous so the result settles as promptly as the cached + // module import it replaces; callers already memoize each pack. + const getBuiltinModule = globalThis.process?.getBuiltinModule; + if (typeof getBuiltinModule !== 'function') + throw new Error( + `God's Eye View needs Node 24.14 or newer to read ${url.pathname.split('/').pop()} (running ${globalThis.process?.version ?? 'an unknown runtime'})`, + ); + const { readFileSync } = getBuiltinModule('node:fs'); + return JSON.parse(readFileSync(url, 'utf8')); + } const response = await fetch(url, { signal }); if (!response.ok) { throw new Error(`HTTP ${response.status} for ${url.pathname}`); diff --git a/src/data/bundledJson.test.mjs b/src/data/bundledJson.test.mjs index 2a7dfe293..1dad604cb 100644 --- a/src/data/bundledJson.test.mjs +++ b/src/data/bundledJson.test.mjs @@ -2,13 +2,14 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { loadBundledJson } from './bundledJson.js'; -const MARINE = new URL('./local_data/natural_earth/marine.json', import.meta.url); -const importMarine = () => - import('./local_data/natural_earth/marine.json', { with: { type: 'json' } }); +const MARINE = new URL( + './local_data/natural_earth/marine.json', + import.meta.url, +); -test('bundled JSON: a file URL uses the JSON import under Node', async (t) => { +test('bundled JSON: a file URL is read from disk under Node', async (t) => { const fetchMock = t.mock.method(globalThis, 'fetch'); - const pack = await loadBundledJson(MARINE, importMarine); + const pack = await loadBundledJson(MARINE); assert.ok(pack.features.some((ft) => ft.name === 'Gulf of Mexico')); assert.equal(fetchMock.mock.callCount(), 0); }); @@ -21,16 +22,62 @@ test('bundled JSON: a served URL is fetched as plain JSON', async (t) => { headers: { 'content-type': 'application/json' }, }); }); - const url = new URL('http://localhost/src/data/local_data/natural_earth/marine.json'); - const importJson = () => assert.fail('the browser path must not import JSON'); - assert.deepEqual(await loadBundledJson(url, importJson), { features: [] }); + const url = new URL( + 'http://localhost/src/data/local_data/natural_earth/marine.json', + ); + assert.deepEqual(await loadBundledJson(url), { features: [] }); assert.deepEqual(requests, [url.href]); }); test('bundled JSON: an HTTP error rejects so the retryable loader can retry', async (t) => { - t.mock.method(globalThis, 'fetch', async () => new Response('', { status: 404 })); + t.mock.method( + globalThis, + 'fetch', + async () => new Response('', { status: 404 }), + ); await assert.rejects( - loadBundledJson(new URL('http://localhost/missing.json'), importMarine), + loadBundledJson(new URL('http://localhost/missing.json')), /HTTP 404/, ); }); + +test('bundled JSON: an aborted load rejects before reading', async () => { + const controller = new AbortController(); + controller.abort(); + await assert.rejects(loadBundledJson(MARINE, { signal: controller.signal }), { + name: 'AbortError', + }); +}); + +test('bundled JSON: no data pack is imported as a module', async () => { + // A literal JSON import in app code makes the production build emit a + // JavaScript copy of the pack beside the fetched JSON, which the browser + // never loads. Tests are not bundled, so they may still import JSON. + const { spawnSync } = await import('node:child_process'); + const result = spawnSync( + 'git', + [ + 'grep', + '-nE', + String.raw`import\(\s*['"][^'"]*local_data/[^'"]*\.json`, + '--', + 'src', + ':!*.test.mjs', + ], + { cwd: new URL('../../', import.meta.url), encoding: 'utf8' }, + ); + // git grep exits 1 when nothing matches. + assert.equal(result.status, 1, result.stdout); +}); + +test('bundled JSON: an older Node says which version is needed', async (t) => { + // Node before 20.16 / 22.3 has no process.getBuiltinModule. + const original = process.getBuiltinModule; + t.after(() => { + process.getBuiltinModule = original; + }); + process.getBuiltinModule = undefined; + await assert.rejects(loadBundledJson(MARINE), { + message: /needs Node 24\.14 or newer to read marine\.json/, + }); +}); diff --git a/src/data/militaryNames.js b/src/data/militaryNames.js index 3f0371bd9..b26ab04e6 100644 --- a/src/data/militaryNames.js +++ b/src/data/militaryNames.js @@ -14,10 +14,6 @@ export function createMilitaryNamesLoader({ loadPack = (signal) => loadBundledJson( new URL('./local_data/osm_military_names/names.json', import.meta.url), - () => - import('./local_data/osm_military_names/names.json', { - with: { type: 'json' }, - }), { signal }, ), timeoutMs = MILITARY_NAMES_TIMEOUT_MS, diff --git a/src/data/naturalEarthRegions.js b/src/data/naturalEarthRegions.js index c4433b5aa..0c7af42d0 100644 --- a/src/data/naturalEarthRegions.js +++ b/src/data/naturalEarthRegions.js @@ -124,17 +124,9 @@ let _entries = null; const PACKS = { regions: { url: new URL('./local_data/natural_earth/regions.json', import.meta.url), - importJson: () => - import('./local_data/natural_earth/regions.json', { - with: { type: 'json' }, - }), }, marine: { url: new URL('./local_data/natural_earth/marine.json', import.meta.url), - importJson: () => - import('./local_data/natural_earth/marine.json', { - with: { type: 'json' }, - }), }, }; @@ -178,8 +170,8 @@ function buildEntries(pack, kind) { */ const loadIndex = createRetryableLoader(async () => { const [regions, marine] = await Promise.all([ - loadBundledJson(PACKS.regions.url, PACKS.regions.importJson), - loadBundledJson(PACKS.marine.url, PACKS.marine.importJson), + loadBundledJson(PACKS.regions.url), + loadBundledJson(PACKS.marine.url), ]); _entries = [ ...buildEntries(regions, 'natural'), diff --git a/src/data/neighborhoodPolygons.js b/src/data/neighborhoodPolygons.js index 2a3c8ca87..61d261d0e 100644 --- a/src/data/neighborhoodPolygons.js +++ b/src/data/neighborhoodPolygons.js @@ -22,10 +22,6 @@ const CITY_FILES = [ './local_data/neighborhoods/san-francisco.json', import.meta.url, ), - importJson: () => - import('./local_data/neighborhoods/san-francisco.json', { - with: { type: 'json' }, - }), }, ]; @@ -112,7 +108,7 @@ function cityLoader(city) { let loader = _cityLoaders.get(city.id); if (!loader) { loader = createRetryableLoader(async () => { - const fc = await loadBundledJson(city.url, city.importJson); + const fc = await loadBundledJson(city.url); return Array.isArray(fc.features) ? fc.features : []; }); _cityLoaders.set(city.id, loader); diff --git a/src/layers/installations/namesLoading.test.mjs b/src/layers/installations/namesLoading.test.mjs index 5b8f6f370..66a5b9148 100644 --- a/src/layers/installations/namesLoading.test.mjs +++ b/src/layers/installations/namesLoading.test.mjs @@ -4,22 +4,47 @@ import { readFileSync } from 'node:fs'; import { createInstallationSource } from './source.js'; import { createIngestion } from './ingestion.js'; import { decodeOpenFreeMapMilitaryTile } from '../../sources/openFreeMap.js'; -import { createMilitaryNamesLoader, loadMilitaryNames } from '../../data/militaryNames.js'; +import { + createMilitaryNamesLoader, + loadMilitaryNames, +} from '../../data/militaryNames.js'; import { loadBundledJson } from '../../data/bundledJson.js'; const box = { south: 30.3, west: -97.8, north: 30.33, east: -97.74 }; const wide = { south: 20, west: -130, north: 60, east: -60 }; -const tile = decodeOpenFreeMapMilitaryTile(readFileSync(new URL('../../data/fixtures/ofm-camp-mabry-12-935-1685.pbf', import.meta.url)), 12, 935, 1685); -const flush = async () => { for (let i = 0; i < 30; i++) await Promise.resolve(); }; -function deferred() { let resolve; const promise = new Promise((r) => { resolve = r; }); return { promise, resolve }; } -function sourceWith(job) { return createInstallationSource({ - loadNames: () => job.promise, - fetchImpl: async () => Response.json({ unavailable: true }), - mapTiles: { fetchBounds: async () => ({ tiles: [tile] }), clear() {} }, -}); } +const tile = decodeOpenFreeMapMilitaryTile( + readFileSync( + new URL( + '../../data/fixtures/ofm-camp-mabry-12-935-1685.pbf', + import.meta.url, + ), + ), + 12, + 935, + 1685, +); +const flush = async () => { + for (let i = 0; i < 30; i++) await Promise.resolve(); +}; +function deferred() { + let resolve; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} +function sourceWith(job) { + return createInstallationSource({ + loadNames: () => job.promise, + fetchImpl: async () => Response.json({ unavailable: true }), + mapTiles: { fetchBounds: async () => ({ tiles: [tile] }), clear() {} }, + }); +} test('close polygons publish immediately while a wide consumer can cancel its independent names wait', async () => { - const job = deferred(), source = sourceWith(job), abort = new AbortController(); + const job = deferred(), + source = sourceWith(job), + abort = new AbortController(); const waiting = source.getMappedSites(wide, { signal: abort.signal }); const rejection = assert.rejects(waiting, { name: 'AbortError' }); const payload = await source.getMappedSites(box); @@ -32,48 +57,81 @@ test('close polygons publish immediately while a wide consumer can cancel its in assert.ok(enriched.records.some((r) => r.name === 'Camp Mabry')); }); -for (const stale of [false, true]) test(`late names ${stale ? 'cannot publish to a superseded view' : 'enrich the current published polygons'}`, async () => { - const job = deferred(), source = sourceWith(job), publications = []; - let currentBox = box; - const state = { enabled: true, viewer: {}, records: [], googleSearchRequested: false }; - const ingestion = createIngestion({ state, source, - services: { render: { governorRequestRender() {} }, ground: { resolveGroundFloorCellsBounded: async () => {} } }, - parts: { - viewport: { loadArea: () => ({ box: currentBox, coverage: {} }), clearUnavailableRetry() {}, scheduleUnavailableRetry() {} }, - model: { installationWithinViewport: () => true }, - rendering: { renderRecords: () => publications.push(state.records), warmInstallationFloors() {} }, - }, +for (const stale of [false, true]) + test(`late names ${stale ? 'cannot publish to a superseded view' : 'enrich the current published polygons'}`, async () => { + const job = deferred(), + source = sourceWith(job), + publications = []; + let currentBox = box; + const state = { + enabled: true, + viewer: {}, + records: [], + googleSearchRequested: false, + }; + const ingestion = createIngestion({ + state, + source, + services: { + render: { governorRequestRender() {} }, + ground: { resolveGroundFloorCellsBounded: async () => {} }, + }, + parts: { + viewport: { + loadArea: () => ({ box: currentBox, coverage: {} }), + clearUnavailableRetry() {}, + scheduleUnavailableRetry() {}, + }, + model: { installationWithinViewport: () => true }, + rendering: { + renderRecords: () => publications.push(state.records), + warmInstallationFloors() {}, + }, + }, + }); + await ingestion.loadInstallations(); + assert.equal(state.loading, false); + assert.ok(publications[0].length); + if (stale) { + currentBox = wide; + state.enabled = false; + state.abort.abort(); + } + job.resolve(await loadMilitaryNames()); + await flush(); + assert.equal(publications.length, stale ? 1 : 2); + if (!stale) assert.ok(state.records.some((r) => r.name === 'Camp Mabry')); + state.abort.abort(); }); - await ingestion.loadInstallations(); - assert.equal(state.loading, false); - assert.ok(publications[0].length); - if (stale) { currentBox = wide; state.enabled = false; state.abort.abort(); } - job.resolve(await loadMilitaryNames()); - await flush(); - assert.equal(publications.length, stale ? 1 : 2); - if (!stale) assert.ok(state.records.some((r) => r.name === 'Camp Mabry')); - state.abort.abort(); -}); -for (const stage of ['response', 'body']) test(`shared names ${stage} deadline aborts and releases the retryable loader`, async (t) => { - t.mock.timers.enable({ apis: ['setTimeout', 'Date'] }); - let calls = 0, signal; - t.mock.method(globalThis, 'fetch', async (_url, options) => { - calls++; signal = options.signal; - if (calls > 1) return Response.json({ records: [], classes: [] }); - if (stage === 'response') return new Promise(() => {}); - return { ok: true, json: () => new Promise(() => {}) }; +for (const stage of ['response', 'body']) + test(`shared names ${stage} deadline aborts and releases the retryable loader`, async (t) => { + t.mock.timers.enable({ apis: ['setTimeout', 'Date'] }); + let calls = 0, + signal; + t.mock.method(globalThis, 'fetch', async (_url, options) => { + calls++; + signal = options.signal; + if (calls > 1) return Response.json({ records: [], classes: [] }); + if (stage === 'response') return new Promise(() => {}); + return { ok: true, json: () => new Promise(() => {}) }; + }); + const load = createMilitaryNamesLoader({ + timeoutMs: 50, + cooldownMs: 100, + loadPack: (signal) => + loadBundledJson(new URL('https://example.test/names.json'), { signal }), + }); + const first = load(); + assert.equal(load(), first, 'shared acquisition'); + const rejected = assert.rejects(first, { name: 'TimeoutError' }); + await flush(); + t.mock.timers.tick(51); + await rejected; + assert.equal(signal.aborted, true); + await assert.rejects(load(), { name: 'TimeoutError' }); + assert.equal(calls, 1); + t.mock.timers.tick(101); + assert.deepEqual((await load()).records, []); + assert.equal(calls, 2); }); - const load = createMilitaryNamesLoader({ timeoutMs: 50, cooldownMs: 100, - loadPack: (signal) => loadBundledJson(new URL('https://example.test/names.json'), null, { signal }) }); - const first = load(); - assert.equal(load(), first, 'shared acquisition'); - const rejected = assert.rejects(first, { name: 'TimeoutError' }); - await flush(); t.mock.timers.tick(51); await rejected; - assert.equal(signal.aborted, true); - await assert.rejects(load(), { name: 'TimeoutError' }); - assert.equal(calls, 1); - t.mock.timers.tick(101); - assert.deepEqual((await load()).records, []); - assert.equal(calls, 2); -});