Skip to content

Commit 3860981

Browse files
committed
Address Claude review feedback
Apply Amazon Location credentials only when a key is configured via env: - Split the Amazon credentials out of getBasemapCredentials() into getAmazonCredentials(), which returns null when no key is set. - getBasemapControlOptions() spreads the Amazon credentials only when present, and the runtime-env-change listener calls setAmazonCredentials() only when a key is configured. This stops an unrelated env change from clobbering an Amazon key entered in the panel's API keys view. - Omit the AWS region when unset instead of hardcoding "us-east-1", so the control keeps its own default region rather than GeoLibre shadowing it.
1 parent 1e3d610 commit 3860981

1 file changed

Lines changed: 34 additions & 17 deletions

File tree

packages/plugins/src/plugins/maplibre-basemap-control.ts

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,33 +30,42 @@ function getRuntimeEnvironment(): Record<string, string | undefined> {
3030
}
3131

3232
/**
33-
* Provider credentials read from runtime environment variables (set in
34-
* Settings → Environment Variables), so users opt in with their own keys. This
35-
* covers the traffic overlays added in maplibre-gl-basemap-control 0.6.0
36-
* (Google/TomTom/HERE; Google Traffic reuses the same VITE_GOOGLE_MAPS_API_KEY
37-
* that the Street View plugin reads) and the Amazon Location styles. Returns
38-
* empty strings when unset, which the control treats as "no key" (the basemap
39-
* then surfaces a "Get a … API key" error rather than loading tiles). The AWS
40-
* region falls back to us-east-1 so Amazon styles work once only the key is set,
41-
* matching the control's own default.
33+
* Traffic-overlay provider credentials (added in maplibre-gl-basemap-control
34+
* 0.6.0) read from runtime environment variables (set in Settings → Environment
35+
* Variables), so users opt in with their own keys. Google Traffic reuses the
36+
* same VITE_GOOGLE_MAPS_API_KEY that the Street View plugin reads. Returns empty
37+
* strings when unset, which the control treats as "no key" (the overlay then
38+
* surfaces a "Get a … API key" error rather than loading tiles).
4239
*/
4340
function getBasemapCredentials(): {
4441
googleMapsApiKey: string;
4542
tomtomApiKey: string;
4643
hereApiKey: string;
47-
amazonApiKey: string;
48-
awsRegion: string;
4944
} {
5045
const env = getRuntimeEnvironment();
5146
return {
5247
googleMapsApiKey: env.VITE_GOOGLE_MAPS_API_KEY?.trim() || "",
5348
tomtomApiKey: env.VITE_TOMTOM_API_KEY?.trim() || "",
5449
hereApiKey: env.VITE_HERE_API_KEY?.trim() || "",
55-
amazonApiKey: env.VITE_AMAZON_LOCATION_API_KEY?.trim() || "",
56-
awsRegion: env.VITE_AMAZON_LOCATION_AWS_REGION?.trim() || "us-east-1",
5750
};
5851
}
5952

53+
/**
54+
* Amazon Location credentials from runtime env, or null when no key is set.
55+
* Unlike the always-applied traffic keys above, these are applied only when a
56+
* key is actually configured, for two reasons: the user's primary way to enter
57+
* an Amazon key is the panel's API keys view (#837), so an unrelated env change
58+
* must not clobber a key typed there; and the region is omitted when unset so
59+
* the control keeps its own default region rather than GeoLibre hardcoding one.
60+
*/
61+
function getAmazonCredentials(): { amazonApiKey: string; awsRegion?: string } | null {
62+
const env = getRuntimeEnvironment();
63+
const amazonApiKey = env.VITE_AMAZON_LOCATION_API_KEY?.trim() || "";
64+
if (!amazonApiKey) return null;
65+
const awsRegion = env.VITE_AMAZON_LOCATION_AWS_REGION?.trim() || undefined;
66+
return awsRegion ? { amazonApiKey, awsRegion } : { amazonApiKey };
67+
}
68+
6069
let basemapControlPosition: GeoLibreMapControlPosition = "top-left";
6170
let removeRuntimeEnvListener: (() => void) | null = null;
6271

@@ -164,8 +173,11 @@ function getBasemapControlOptions(
164173
// Provider basemaps that need a key (the Google/TomTom/HERE traffic overlays
165174
// and the Amazon Location styles) authenticate with the user's own
166175
// credentials, read from runtime env. Unset keys are harmless: the basemap
167-
// just reports a missing-key error instead of loading tiles.
176+
// just reports a missing-key error instead of loading tiles. Amazon is
177+
// spread only when its key is set, so an unset key leaves the control's own
178+
// region default in place.
168179
...getBasemapCredentials(),
180+
...(getAmazonCredentials() ?? {}),
169181
// A style basemap (e.g. OpenFreeMap 3D) swaps the whole map style and so
170182
// discards every stacked raster basemap. In stack mode that silently wiped
171183
// a carefully assembled stack, so confirm before the rasters are lost. See
@@ -194,12 +206,17 @@ function addRuntimeEnvListener(): void {
194206

195207
const handleRuntimeEnvChange = () => {
196208
if (!basemapControl) return;
197-
const { googleMapsApiKey, tomtomApiKey, hereApiKey, amazonApiKey, awsRegion } =
198-
getBasemapCredentials();
209+
const { googleMapsApiKey, tomtomApiKey, hereApiKey } = getBasemapCredentials();
199210
basemapControl.setGoogleMapsApiKey(googleMapsApiKey);
200211
basemapControl.setTomTomApiKey(tomtomApiKey);
201212
basemapControl.setHereApiKey(hereApiKey);
202-
basemapControl.setAmazonCredentials(amazonApiKey, awsRegion);
213+
// Only push Amazon credentials when a key is configured via env, so an
214+
// unrelated env change never clears a key entered in the panel. The region
215+
// is left undefined when unset, keeping the control's own default.
216+
const amazon = getAmazonCredentials();
217+
if (amazon) {
218+
basemapControl.setAmazonCredentials(amazon.amazonApiKey, amazon.awsRegion);
219+
}
203220
};
204221

205222
window.addEventListener("geolibre:runtime-env-change", handleRuntimeEnvChange);

0 commit comments

Comments
 (0)