Both comments were right, and the second named a real blind spot.
The e2e iterated a hardcoded theme list while the unit suite iterated
`themeNames`, so a fifth theme would have got mapping coverage for free
and no cold start. It imports `themeNames` and `themeSessionKey` now.
The sprite was invisible to it. A wrong sprite name 404s the icons and
MapLibre carries on, so the canvas exists and the suite passes -- exactly
the case the review described. The spec now intercepts
`/map-assets/sprites/**` and asserts the requested set is `mapSprite()`.
Planted it by collapsing the sprite back into the flavor: night and amber
get PAST the canvas assertion and fail on the sprite, which is the point.
The rebuild branch cannot be reached from a browser at all. MapViewer
builds a style twice -- at init, and in an effect keyed on
`[theme, mapReady, scaleUnit]` -- but the theme picker lives on Settings
and in the gallery, never in the nav, so there is no way to change the
theme with /map open; navigating to Settings unmounts the viewer. A
Cypress case that switches themes would have nothing to click.
So that branch is guarded in the source instead: both `buildStyle` calls
must pass `mapFlavor(theme)` and `mapSprite(theme)`, and none may pass a
bare `theme`. Crude, and the honest alternative to a browser test that
cannot reach the branch it claims to cover. Planted by breaking ONLY the
rebuild call: the source guards go red and the e2e stays green, which is
the whole argument for keeping it.
The call-site scan counts parens rather than matching a regex. The
arguments are themselves calls, so anything non-greedy stops at
`mapFlavor(theme)` and reports half an argument list as the whole of it --
which is how the first version of this test failed while the code was
correct.
1098 jest / 61 suites, 4 e2e themes, clean build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MapViewer handed protomaps-themes-base the WROLPi theme name as its
basemap "flavor". That worked while the only themes were `light` and
`dark`, and broke the day night and amber arrived: protomaps looks the
flavor up in a record and dereferences the result without checking, so an
unknown name is not a fallback but
TypeError: Cannot read properties of undefined (reading 'background')
at nolabels_layers (base_layers.ts:18)
thrown before the first tile. MapLibre never constructed, so /map
rendered nothing in two of the four themes.
`mapFlavor()` in themes/names.ts maps a theme to one of the two flavors we
can actually serve. Only `light` and `dark`: the flavor is interpolated
into `/map-assets/sprites/<flavor>` too, and those are the two sprite sets
we ship, so protomaps' `black` and `grayscale` would load a style and then
find no icons. Night and amber take `dark` and let the media filter tint
the canvas -- a filter shifts hue, so handing them `light` would give a
red or amber slab, which is the opposite of the point.
Two layers of test, because they cover different halves:
- map-flavor.test.js checks every theme against the REAL protomaps
(calling layers() and requiring it not to throw) and against the
sprites directory on disk, rather than a list of valid names copied
into the test, which is what would have passed here in the first
place. It fails when a fifth theme is added without a mapping.
- map-themes.cy.js drives the actual page in all four themes and waits
for the MapLibre canvas. The unit test cannot see whether MapViewer
USES the mapping, which is the half that was broken. Verified against
the planted regression: night and amber fail with no canvas, light and
dark still pass.
Not a bug: the 404 on /blobs/map-overview.pmtiles in the same trace. That
is a HEAD probe for an optional planet overview, gated on `resp.ok`, and
the blob is absent in a dev checkout. The console line is unavoidable.
1093 jest / 61 suites, clean build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
