diff --git a/internal/check-facet-shape.mjs b/internal/check-facet-shape.mjs index 8df2d2999..ed3aa2b1f 100644 --- a/internal/check-facet-shape.mjs +++ b/internal/check-facet-shape.mjs @@ -6,6 +6,7 @@ * EXIT 0 pass · 1 fail */ import { WALK } from './lib/facet-walk.mjs' +import { readSourceFeatures } from '../library/maps/pin-selection.js' // ── SHAPE TEST (headless, no browser) ──────────────────────────────────────────────────────────── // ★ NEGATIVE-TESTS __srcFeatures AGAINST BOTH ENGINE SHAPES, in sr's check-pin-selection style. @@ -25,8 +26,14 @@ const cases = [ // drew nothing", which is the finding this gate exists to report. ['remote (_data = { url }) -> null, not 0', () => srcFeatures(mapWith({ _data: { url: 'https://x.invalid/a.json' } }), 'rd-bubbles'), null], ['absent source -> null', () => srcFeatures(mapWith(null), 'rd-bubbles'), null], + // ★ maplibre's THIRD shape. updateData() stores _data = { updateable } (maplibre-gl :49001). Nothing + // calls updateData today, so this fixture is here to make ADOPTING it trip a test instead of blanking + // the map: neither reader understands the shape, and the one thing both must not do is answer 0. + ['maplibre updateData (_data = { updateable }) -> null, not 0', + () => srcFeatures(mapWith({ _data: { updateable: new Map() } }), 'rd-bubbles'), null], ] let ok = 0 +let total = cases.length for (const [name, run, want] of cases) { const got = run() const n = got === null || got === undefined ? null : got.length @@ -34,7 +41,68 @@ for (const [name, run, want] of cases) { console.log(` ${pass ? '✓' : '✗'} ${name} -> ${n === null ? 'null' : n + ' feature(s)'}`) if (pass) ok++ } -console.log(ok === cases.length - ? `FACET-SHAPE-TEST: PASS — ${ok}/${cases.length}, both engine shapes measured` - : `FACET-SHAPE-TEST: FAIL — ${ok}/${cases.length}`) -process.exit(ok === cases.length ? 0 : 1) +// ── THE TWO READERS MUST NOT DRIFT ─────────────────────────────────────────────────────────────── +// This gate's helper (__srcFeatures, injected into the page) and the PRODUCT's reader +// (library/maps/pin-selection.js readSourceFeatures, used by the packet and zoomToLegendRow) answer the +// same question about the same private field, and they are two separate copies because one must be a +// string evaluated in a browser and the other an ES module. Nothing but this block holds them together. +// +// ★ ONE DIFFERENCE IS DELIBERATE AND IS ASSERTED AS SUCH, not waived. On a url-backed source the gate +// returns null ("cannot measure") while the product returns [] and warns. Both are right for their +// caller: a gate whose whole job is to distinguish "the map drew nothing" from "I could not look" must +// have a third answer, while a selection returning null would break the Set arithmetic every caller +// does on it. Pinning the fork here means the next person to touch either reader is told that it is a +// decision, and any OTHER disagreement is drift and fails. +const agree = [ + ['mapbox (_data = FeatureCollection)', { _data: FC }, 2], + ['maplibre (_data = { geojson })', { _data: { geojson: FC } }, 2], + ['serialize() fallback', { serialize: () => ({ data: FC }) }, 2], + ['single Feature, not a collection', { _data: { type: 'Feature', properties: { slug: 'a' } } }, 1], + ['unrecognised shape ({ updateable })', { _data: { updateable: new Map() } }, 0], +] +for (const [name, src, wantCount] of agree) { + const a = srcFeatures(mapWith(src), 'rd-bubbles') + const b = readSourceFeatures(mapWith(src), 'rd-bubbles') + const na = a === null || a === undefined ? 0 : a.length + const nb = b === null || b === undefined ? 0 : b.length + const pass = na === nb && na === wantCount + console.log(` ${pass ? '✓' : '✗'} both readers agree: ${name} -> gate ${na}, product ${nb}`) + if (pass) ok++ + total++ +} +{ + // The documented fork, asserted in BOTH directions so neither side can quietly adopt the other's. + const src = { _data: { url: 'https://x.invalid/a.json' } } + const a = srcFeatures(mapWith(src), 'rd-bubbles') + const b = readSourceFeatures(mapWith(src), 'rd-bubbles') + const pass = a === null && Array.isArray(b) && b.length === 0 + console.log(` ${pass ? '✓' : '✗'} documented fork on a url source: gate null ("cannot measure"), product [] + warn`) + if (pass) ok++ + total++ +} +// ── AND NEITHER MAY BE SILENT ABOUT A SHAPE IT DOES NOT KNOW ──────────────────────────────────── +// "Returns 0" and "returns 0 while saying it did not understand the payload" are different products. +// Only the second lets someone adopting updateData() find out from a console instead of from a map. +for (const [label, run] of [ + ['gate helper', () => srcFeatures(mapWith({ _data: { updateable: new Map() } }), 'rd-bubbles')], + // A map whose getSource answers to ANY id: mapWith() only knows 'rd-bubbles', so asking it for a + // fresh id returns no source at all and the reader exits before it can warn — the assertion would + // have passed for the wrong reason on a reader that never warns. The fresh id is still needed, + // because the warning is once-per-source by design. + ['product reader', () => readSourceFeatures( + { getSource: () => ({ _data: { updateable: new Map() } }) }, 'a-source-not-yet-warned-about')], +]) { + const said = [] + const real = console.warn + console.warn = (...a) => said.push(a.join(' ')) + try { run() } finally { console.warn = real } + const pass = said.some((m) => /updateable/.test(m)) + console.log(` ${pass ? '✓' : '✗'} ${label} names the unrecognised shape rather than failing silently`) + if (pass) ok++ + total++ +} + +console.log(ok === total + ? `FACET-SHAPE-TEST: PASS — ${ok}/${total}, both engine shapes measured, the gate's reader and the product's reader agree everywhere except their one documented fork, and neither is silent about a shape it does not know` + : `FACET-SHAPE-TEST: FAIL — ${ok}/${total}`) +process.exit(ok === total ? 0 : 1) diff --git a/internal/lib/facet-walk.mjs b/internal/lib/facet-walk.mjs index 4e6cdac03..899c92db3 100644 --- a/internal/lib/facet-walk.mjs +++ b/internal/lib/facet-walk.mjs @@ -57,7 +57,22 @@ function __srcFeatures(m, id) { // catch. null means "could not measure", and the caller must not treat it as zero. else if (data.url !== undefined) return null; } - return (data && data.features) || null; + if (data && data.features) return data.features; + // A SOURCE MAY HOLD ONE FEATURE, NOT A COLLECTION — both engines accept it, and the product's reader + // already did. This helper did not, so it answered "cannot measure" for a payload it could read + // perfectly well. Found by the cross-check in check-facet-shape, not by anyone reading either file. + if (data && data.type === 'Feature') return [data]; + // ★ AN UNRECOGNISED SHAPE MUST SAY SO BY NAME, not read as "the map drew nothing". maplibre has a + // THIRD _data shape this reader does not handle: updateData() stores { updateable } (maplibre-gl + // :49001), and nothing in the product calls updateData today — so the day someone adopts partial + // updates for performance, this would have gone quietly back to reporting an empty map about a map + // full of dots, which is the exact defect this gate exists to report. Naming the keys makes the + // next reader's first question ("what shape IS it?") answered by the message itself. + try { + console.warn('[facet-walk] source "' + id + '": unrecognised _data shape {' + + Object.keys(data || {}).join(',') + '} — cannot measure, NOT zero features'); + } catch (e) {} + return null; } function __live() { const m = window.__rdashMap; diff --git a/library/maps/pin-selection.js b/library/maps/pin-selection.js index bbc853514..19ba453b0 100644 --- a/library/maps/pin-selection.js +++ b/library/maps/pin-selection.js @@ -163,6 +163,18 @@ export function readSourceFeatures(map, sourceId) { if (!data) return []; if (Array.isArray(data?.features)) return data.features; if (data?.type === 'Feature') return [data]; + // ★ THE SAME RULE AS THE url BRANCH, FOR THE SHAPES WE HAVE NOT MET YET. Falling off the end used + // to return [] in silence, which is "the map drew nothing" stated as fact about a payload nobody + // recognised — the precise failure the url warning above exists to prevent, left in place one line + // below it. maplibre already HAS a third shape: updateData() stores _data = { updateable } + // (maplibre-gl :49001). No product code calls updateData today, so this is a landmine rather than + // a bug, and it is armed the moment someone adopts partial updates. Warned once per source, with + // the keys named, so the message identifies the shape instead of merely reporting a miss. + if (data && typeof data === 'object' && !_warnedUrlSources.has(sourceId)) { + _warnedUrlSources.add(sourceId); + // eslint-disable-next-line no-console + console.warn(`[pin-selection] source "${sourceId}": unrecognised data shape {${Object.keys(data).join(',')}} — nothing can be selected from it; this is NOT a confirmed empty source.`); + } return []; }