Merge pull request #901 from bilawalsidhu/fix/build-hygiene

build: ship each bundled data pack once

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

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 = {

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<any>} 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}`);

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/,
+  });
+});

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,

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'),

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);

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);
-});