Two readers of one private field, and nothing was holding them together
internal/lib/facet-walk.mjs's __srcFeatures (injected into the page by
check:facet-changes-map) and library/maps/pin-selection.js's readSourceFeatures (the
packet and zoomToLegendRow) answer the same question about the same private GeoJSON
field. They are two copies because one must be a string evaluated in a browser and the
other an ES module, so they cannot be one function -- but nothing tested that they
still agreed, and they had already stopped.
THE CROSS-CHECK FOUND A DIVERGENCE ON ITS FIRST RUN, which is the argument for it. A
source may hold a single Feature rather than a FeatureCollection; both engines accept
that. The product's reader handled it and the gate's did not, so the gate answered
"cannot measure" about a payload it could read perfectly well. Nobody reading either
file would have noticed: each is correct on its own terms. __srcFeatures now handles
it and the two agree.
ONE DIFFERENCE IS DELIBERATE AND IS NOW PINNED AS SUCH RATHER THAN WAIVED: on a
url-backed source the gate returns null while the product returns [] and warns. Both
are right for their caller -- a gate whose job is to tell "the map drew nothing" from
"I could not look" needs a third answer, while a selection returning null would break
the Set arithmetic every caller does on it. Asserted in both directions, so neither
side can quietly adopt the other's and call it a fix.
AND NEITHER MAY BE SILENT ABOUT A SHAPE IT DOES NOT KNOW. maplibre has a THIRD _data
shape: updateData() stores { updateable } (maplibre-gl-dev.js: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 for performance, at which point the product
reader would have gone back to returning [] in silence: "the map drew nothing", stated
as fact about a payload nobody recognised, one line below the url warning that exists
to prevent exactly that. Both readers now name the unrecognised shape by its keys, and
a fixture holds it, so adopting updateData() trips a test instead of blanking a map.
Negative-tested: dropping the single-Feature case, silencing either reader, or making
the gate adopt the product's url answer each turn it red; restored, 14/14 green.
check:pin-selection still passes, so the product reader is unchanged where it matters.
This commit is contained in:
parent
722ac77892
commit
42b4c8f16b
3 changed files with 100 additions and 5 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 [];
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue