fix(atlas): retile the map instead of rebuilding it when the CARTO key lands

useTileUrl returns a new string once carto_api_key reaches the settings
store, which happens after the first render. That string was a dependency
of the effect that *builds* the Leaflet map, so the key arriving tore the
map down and built a new one.

Two things break in that window: Leaflet finishes a queued redraw against
the canvas renderer whose map has gone ("Cannot read properties of
undefined (reading 'save')"), and the country layer never comes back —
its effect only re-runs when its own data changes, so the rebuilt map
shows tiles and nothing else.

The template belongs to the tile layer, not to the map. Hold the layers
in a ref, read the template through a ref inside the build effect, and
setUrl them from an effect of their own. Same fix in JourneyMap, where
cartoKey and mapTileUrl sat in the dep arrays of the effects that draw
every marker and track.

Fixes #2097
This commit is contained in:
Konstantinos Thermos
2026-08-28 15:28:08 +03:00
committed by Maurice
parent 5a7b8d75aa
commit 1c5c3ae983
5 changed files with 91 additions and 13 deletions
@@ -37,7 +37,7 @@ vi.mock('leaflet', () => {
return {
default: {
map: vi.fn(() => mockMap),
tileLayer: vi.fn(() => ({ addTo: vi.fn() })),
tileLayer: vi.fn(() => ({ addTo: vi.fn(), setUrl: vi.fn() })),
marker: vi.fn(() => mockMarker),
polyline: vi.fn(() => { const line: any = { addTo: vi.fn(() => line), bindTooltip: vi.fn(() => line) }; return line }),
divIcon: vi.fn(() => ({})),
@@ -45,7 +45,7 @@ vi.mock('leaflet', () => {
layerGroup: vi.fn(() => ({ addLayer: vi.fn(), addTo: vi.fn(), remove: vi.fn() })),
},
map: vi.fn(() => mockMap),
tileLayer: vi.fn(() => ({ addTo: vi.fn() })),
tileLayer: vi.fn(() => ({ addTo: vi.fn(), setUrl: vi.fn() })),
marker: vi.fn(() => mockMarker),
polyline: vi.fn(() => { const line: any = { addTo: vi.fn(() => line), bindTooltip: vi.fn(() => line) }; return line }),
divIcon: vi.fn(() => ({})),
@@ -388,6 +388,22 @@ describe('JourneyMap', () => {
expect(vi.mocked(L.tileLayer).mock.calls[0][0]).toBe('https://tiles.test/{z}/{x}/{y}.png');
});
it('FE-COMP-JOURNEYMAP-041: a CARTO key arriving after mount retiles instead of rebuilding the map (#2097)', () => {
render(<JourneyMap checkins={[]} entries={entriesWithCoords} />);
const tiles = vi.mocked(L.tileLayer).mock.results[0].value as { setUrl: ReturnType<typeof vi.fn> };
const mapsBefore = vi.mocked(L.map).mock.calls.length;
act(() => {
seedStore(useSettingsStore, { settings: buildSettings({ carto_api_key: 'k1' }) });
});
expect(tiles.setUrl).toHaveBeenCalledWith(expect.stringContaining('key=k1'));
// The markers and tracks the map already carries are drawn by the effect that
// builds it, so a rebuild here would tear them down mid-flight.
expect(vi.mocked(L.map).mock.calls.length).toBe(mapsBefore);
expect(mockedMap().remove).not.toHaveBeenCalled();
});
it('FE-COMP-JOURNEYMAP-028: the activeMarkerId prop flies to that marker after the settle delay', () => {
vi.useFakeTimers();
try {
+20 -4
View File
@@ -154,6 +154,13 @@ function JourneyMap(
const mapTileUrl = useSettingsStore(s => s.settings.map_tile_url)
const storedCartoKey = useCartoApiKey()
const cartoKey = cartoApiKey || storedCartoKey
const tileUrl = resolveTileUrl(mapTileUrl, dark ? CARTO_DARK : CARTO_VOYAGER, cartoKey)
// Read through a ref by the map effect, retiled in place by its own effect below:
// the CARTO key reaches the store after the first render, and rebuilding the map
// for that raced with the markers and layers already on it (#2097).
const tileUrlRef = useRef(tileUrl)
tileUrlRef.current = tileUrl
const tileLayerRef = useRef<L.TileLayer | null>(null)
const containerRef = useRef<HTMLDivElement>(null)
const mapRef = useRef<L.Map | null>(null)
const markersRef = useRef<Map<string, L.Marker>>(new Map())
@@ -232,7 +239,7 @@ function JourneyMap(
})
mapRef.current = map
L.tileLayer(resolveTileUrl(mapTileUrl, dark ? CARTO_DARK : CARTO_VOYAGER, cartoKey), {
const tiles = L.tileLayer(tileUrlRef.current, {
maxZoom: 18,
attribution: '&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a>',
referrerPolicy: 'strict-origin-when-cross-origin',
@@ -242,7 +249,9 @@ function JourneyMap(
// updates and keep a larger ring of off-screen tiles ready.
updateWhenIdle: false,
keepBuffer: 4,
} as any).addTo(map)
} as any)
tiles.addTo(map)
tileLayerRef.current = tiles
const items = buildMarkerItems(entries)
itemsRef.current = items
@@ -337,9 +346,16 @@ function JourneyMap(
return () => {
map.remove()
mapRef.current = null
tileLayerRef.current = null
markersRef.current.clear()
}
}, [entries, stableTrail, stableTracks, dark, mapTileUrl, cartoKey, fullScreen, paddingBottom])
}, [entries, stableTrail, stableTracks, dark, fullScreen, paddingBottom])
// Retile in place rather than through the effect above, which would drop every
// marker and track it just drew.
useEffect(() => {
tileLayerRef.current?.setUrl(tileUrl)
}, [tileUrl])
// Photo layer (#1614). Its own effect on purpose: photos arriving must not tear
// down and rebuild the map the way the entry effect does. Redrawn on zoom and
@@ -382,7 +398,7 @@ function JourneyMap(
photoLayerRef.current?.remove()
photoLayerRef.current = null
}
}, [photos, entries, stableTrail, stableTracks, dark, mapTileUrl, cartoKey, fullScreen, paddingBottom])
}, [photos, entries, stableTrail, stableTracks, dark, fullScreen, paddingBottom])
// react to activeMarkerId prop changes — runs after map is built
useEffect(() => {
+1 -1
View File
@@ -71,7 +71,7 @@ vi.mock('leaflet', () => {
const L = {
map: vi.fn(() => mockMap),
tileLayer: vi.fn(() => ({ addTo: vi.fn().mockReturnThis() })),
tileLayer: vi.fn(() => ({ addTo: vi.fn().mockReturnThis(), setUrl: vi.fn() })),
// Call onEachFeature and style callbacks for each feature so those paths are covered
geoJSON: vi.fn((data: any, options: any) => {
if (options?.onEachFeature && data?.features) {
+29 -1
View File
@@ -105,7 +105,7 @@ vi.mock('leaflet', () => {
const L = {
map: vi.fn(() => { lf.mapsCreated += 1; return map; }),
tileLayer: vi.fn(() => ({ addTo: vi.fn() })),
tileLayer: vi.fn(() => ({ addTo: vi.fn(), setUrl: vi.fn() })),
control: { zoom: vi.fn(() => ({ addTo: vi.fn() })) },
canvas: vi.fn(() => ({})),
svg: vi.fn(() => ({})),
@@ -708,6 +708,34 @@ describe('useAtlas', () => {
expect(lf.mapsRemoved).toBeGreaterThan(0);
});
it('FE-HOOK-ATLAS-043: a CARTO key arriving after mount retiles instead of rebuilding the map (#2097)', async () => {
// Sliced from here: L.tileLayer's mock results carry over from earlier tests.
const madeBefore = vi.mocked(L.tileLayer).mock.results.length;
await mountAtlas({ geo: geoCountries });
await waitFor(() => expect(lf.geoJson.length).toBeGreaterThan(0));
const tiles = vi.mocked(L.tileLayer).mock.results
.slice(madeBefore)
.map((r) => r.value as { setUrl: ReturnType<typeof vi.fn> });
expect(tiles.length).toBeGreaterThan(0);
const layersBefore = lf.geoJson.length;
act(() => {
seedStore(useSettingsStore, { settings: buildSettings({ dark_mode: false, carto_api_key: 'k1' }) });
});
// The key reaches the tiles that are already on the map...
await waitFor(() => {
for (const t of tiles) expect(t.setUrl).toHaveBeenCalledWith(expect.stringContaining('key=k1'));
});
// ...and nothing else is torn down: rebuilding the map dropped the country layer
// (only redrawn when its own data changes) and left Leaflet redrawing a canvas
// renderer whose map had gone.
expect(lf.mapsCreated).toBe(1);
expect(lf.mapsRemoved).toBe(0);
expect(lf.geoJson.length).toBe(layersBefore);
});
it('FE-HOOK-ATLAS-027: a plugin layer naming no country in the map draws nothing', async () => {
await mountAtlas({
geo: geoCountries,
+23 -5
View File
@@ -84,6 +84,12 @@ export function useAtlas() {
// Label-free tiles on purpose (the country fills carry the names here), so the
// user's own template is deliberately not read, only their CARTO key.
const tileUrl = useTileUrl(dark ? CARTO_DARK_NOLABELS : CARTO_LIGHT_NOLABELS, true)
// The template is read through a ref inside the map effect so a template change —
// the CARTO key arriving after the first render is the usual one — retiles the
// layers below instead of tearing the whole map down and building it again (#2097).
const tileUrlRef = useRef(tileUrl)
tileUrlRef.current = tileUrl
const tileLayersRef = useRef<L.TileLayer[]>([])
const mapRef = useRef<HTMLDivElement>(null)
const mapInstance = useRef<L.Map | null>(null)
const geoLayerRef = useRef<L.GeoJSON | null>(null)
@@ -367,7 +373,7 @@ export function useAtlas() {
L.control.zoom({ position: 'bottomright' }).addTo(map)
L.tileLayer(tileUrl, {
const baseTiles = L.tileLayer(tileUrlRef.current, {
maxZoom: 10,
keepBuffer: 25,
updateWhenZooming: true,
@@ -376,17 +382,20 @@ export function useAtlas() {
zoomOffset: 0,
crossOrigin: true,
referrerPolicy: 'strict-origin-when-cross-origin',
} as any).addTo(map)
} as any)
baseTiles.addTo(map)
// Preload adjacent zoom level tiles
L.tileLayer(tileUrl, {
const preloadTiles = L.tileLayer(tileUrlRef.current, {
maxZoom: 10,
keepBuffer: 10,
opacity: 0,
tileSize: 256,
crossOrigin: true,
referrerPolicy: 'strict-origin-when-cross-origin',
}).addTo(map)
})
preloadTiles.addTo(map)
tileLayersRef.current = [baseTiles, preloadTiles]
// Custom pane for region layer — above overlay (z-index 400)
map.createPane('regionPane')
@@ -443,8 +452,17 @@ export function useAtlas() {
// appending a second container to the pane.
regionLayerRef.current = null
renderedRegionSigRef.current = ''
tileLayersRef.current = []
}
}, [dark, loading, tileUrl])
}, [dark, loading])
// Retile in place. A rebuild would drop every layer the effects below hold a ref
// to — the country layer is only re-rendered when its own data changes, so the map
// would come back bare — and Leaflet keeps redrawing the torn-down canvas renderer
// for a frame after the map goes (#2097).
useEffect(() => {
for (const layer of tileLayersRef.current) layer.setUrl(tileUrl)
}, [tileUrl])
// Render GeoJSON countries
useEffect(() => {