Migrate Google Maps integration to @vis.gl/react-google-maps#1619
Migrate Google Maps integration to @vis.gl/react-google-maps#1619kaligrafy wants to merge 1 commit into
Conversation
WalkthroughThis PR migrates Google map widgets from Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kaligrafy
requested review from
samuel-duhaime and
tahini
and removed request for
tahini
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsx`:
- Around line 30-42: When creating overlays in the map-mount effect (the
React.useEffect that runs when map becomes non-null) include the current options
so initial style/path isn't missed: construct the Polyline/overlay with both map
and options or call polylineRef.current.setOptions(options) immediately after
assigning polylineRef.current. Update the effect that creates the overlay (the
one that constructs google.maps.Polyline and assigns polylineRef.current) to
apply the provided options from the options variable (and do the same for any
other overlay creation effects around the other block that creates overlays),
ensuring polylineRef.current is initialized with options on mount.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 53e94673-809a-49b1-b577-75d61088b281
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.env.examplepackages/evolution-frontend/package.jsonpackages/evolution-frontend/src/components/inputs/__tests__/InputMapFindPlace.test.tsxpackages/evolution-frontend/src/components/inputs/__tests__/InputMapPoint.test.tsxpackages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsxpackages/evolution-frontend/src/config/googleMaps.config.tspackages/evolution-frontend/src/utils/dev/webpackCommon.tspackages/evolution-frontend/tsconfig.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test-sequential (24.x)
- GitHub Check: check-format
- GitHub Check: build-and-test (24.x)
- GitHub Check: generate-and-build-generator (24.x)
- GitHub Check: pr-build-check
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-04-10T18:04:30.397Z
Learnt from: tahini
Repo: chairemobilite/evolution PR: 1491
File: packages/evolution-common/src/services/questionnaire/sections/visitedPlaces/widgetNextPlaceCategory.ts:25-31
Timestamp: 2026-04-10T18:04:30.397Z
Learning: In `packages/evolution-common/src/services/odSurvey/helpers.ts`, ensure `getHomeAddressOneLine` and the internal `getAddressOneLine` HTML-escape every address field (e.g., via lodash `_escape`) before assembling the returned one-line HTML string. Put the escaping in these helpers so all callers automatically receive a safe HTML string, and avoid requiring individual call sites to perform escaping.
Applied to files:
.env.example
📚 Learning: 2026-04-17T16:19:46.635Z
Learnt from: samuel-duhaime
Repo: chairemobilite/evolution PR: 1505
File: packages/evolution-generator/src/scripts/generate_choices.py:189-202
Timestamp: 2026-04-17T16:19:46.635Z
Learning: In packages/evolution-generator/src/scripts/generate_choices.py, when generating i18next translation keys for choices, do not escape dots in the choice value. Build translation keys using the literal format `choices:<choiceName>.<value>` (e.g., `choices:<choiceName>.some.value`) so that dot-containing values remain unmodified and match the existing i18next key convention. Do not add extra escaping logic just to handle dots.
Applied to files:
.env.example
📚 Learning: 2026-03-31T18:20:32.322Z
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1462
File: packages/evolution-common/src/services/baseObjects/Completable.ts:1-6
Timestamp: 2026-03-31T18:20:32.322Z
Learning: In this repository (chairemobilite/evolution), do not flag or require a copyright year in file headers. For new files, the header format `Copyright Polytechnique Montreal and contributors` (without a year) is acceptable. Only verify the header presence/consistency, not the presence of the year.
Applied to files:
packages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsxpackages/evolution-frontend/src/utils/dev/webpackCommon.tspackages/evolution-frontend/src/config/googleMaps.config.tspackages/evolution-frontend/src/components/inputs/__tests__/InputMapFindPlace.test.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsxpackages/evolution-frontend/src/components/inputs/__tests__/InputMapPoint.test.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsx
🔇 Additional comments (9)
packages/evolution-frontend/src/components/inputs/__tests__/InputMapFindPlace.test.tsx (1)
158-338: LGTM!packages/evolution-frontend/src/components/inputs/__tests__/InputMapPoint.test.tsx (1)
9-9: LGTM!Also applies to: 30-30, 36-36, 44-46, 58-58, 75-77, 88-88, 90-90, 110-112, 136-176
.env.example (1)
21-27: LGTM!packages/evolution-frontend/package.json (1)
40-40: LGTM!packages/evolution-frontend/tsconfig.json (1)
8-11: LGTM!packages/evolution-frontend/src/utils/dev/webpackCommon.ts (1)
113-113: LGTM!packages/evolution-frontend/src/config/googleMaps.config.ts (1)
9-20: LGTM!Also applies to: 25-44, 46-54
packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx (1)
2-27: LGTM!Also applies to: 33-345
packages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsx (1)
2-27: LGTM!Also applies to: 35-44, 48-49, 66-110, 112-217, 260-340
Reviewer / deployer note: setting up a Google Map IDTo take advantage of the new vector maps and How to create one
Official docs: What happens without a Map IDThe widgets still work — they fall back to the legacy raster RestrictionsWhen creating the Map ID, you can restrict it to specific HTTP referrers (same as the API key) to prevent reuse on other domains. Recommended for production. |
kaligrafy
force-pushed
the
migrateGoogleMapsLib
branch
from
1fb28c1 to
97ba2c2
Compare
tahini
left a comment
tahini
left a comment
There was a problem hiding this comment.
Quelques commentaires, globalement, ça me semble bien, si ça a été bien testé.
1- Svp squasher les commits dans un seul, ils ne semblent pas indépendants. Par exemple, le commit qui ne fait que swapper les packages ne doit pas fonctionner tout seul...
2- Ajouter dans le CHANGELOG.md un one-liner qui explique les implications de ça (créer un nouvel ID changer une valeur dans le .env, on pourra éventuellement faire un lien au commit ID une fois la PR mergée, si le commit a plus de détails). Par exemple, faudra-t-il donner des instructiosn spécifiques à nos partenaires qui ont l'habitude de nous fournir une clé Google?
3- Aussi ajouter au changelog.md, sous dev le changement de package.
Replaces the deprecated @react-google-maps/api package with @vis.gl/react-google-maps and rewrites the Google map widgets (InputMapGoogle, InfoMapGoogle) on top of <APIProvider>/<Map> and the useMap()/useApiIsLoaded() hooks (fixes chairemobilite#453). Maps run uncontrolled (defaultCenter) and recenter via map.panTo() so user pan is no longer fought by re-renders. Markers always use <AdvancedMarker>; the legacy <Marker> fallback was dropped, so a Google Map ID is now required. Polyline/Polygon are not provided by @vis.gl/react-google-maps, so InfoMap renders them through small imperative helpers (MapPolyline/MapPolygon) mounted via useMap(), initialized with their options on mount. Config / build: - googleMaps.config.ts exposes the APIProvider config and getGoogleMapId(). - webpackCommon forwards GOOGLE_MAP_ID into the frontend bundle. - tsconfig includes @types/google.maps (was transitive in the old package). - .env.example documents the now-required GOOGLE_MAP_ID with official links. Tests: - Parameterize the geocoding flows in InputMapPoint and InputMapFindPlace and share a renderWidget helper, while keeping the original inline comments. - Add a case covering "user picks a non-default option from the multi-result list, confirms, and the chosen place_id is what gets saved" (legal compliance: place_id must be preserved).
kaligrafy
force-pushed
the
migrateGoogleMapsLib
branch
from
97ba2c2 to
e63981d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/evolution-frontend/src/utils/dev/webpackCommon.ts (1)
107-114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
params.envforGOOGLE_MAP_IDtoo.
CommonWebpackConfigParams.envis documented to overrideprocess.env, but this new entry ignores it. Builds that pass the map id through the webpack helper/env arg will silently missmapId.Suggested fix
'process.env': { IS_BROWSER: JSON.stringify(true), HOST: JSON.stringify(process.env.HOST), APP_NAME: JSON.stringify('survey'), IS_TESTING: JSON.stringify(currentNodeEnv === 'test'), GOOGLE_API_KEY: JSON.stringify(process.env.GOOGLE_API_KEY), - GOOGLE_MAP_ID: JSON.stringify(process.env.GOOGLE_MAP_ID), + GOOGLE_MAP_ID: JSON.stringify(params.env.GOOGLE_MAP_ID ?? process.env.GOOGLE_MAP_ID), ...stringifyEnvValues(params.extraEnvs) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evolution-frontend/src/utils/dev/webpackCommon.ts` around lines 107 - 114, The build injects GOOGLE_MAP_ID from process.env but should honor CommonWebpackConfigParams.env overrides; update the 'process.env' mapping for GOOGLE_MAP_ID in webpackCommon.ts to read from params.env first (e.g. use params.env?.GOOGLE_MAP_ID or equivalent) and fall back to process.env.GOOGLE_MAP_ID, ensuring the value is JSON.stringify'd like the other entries; reference the mapping block that includes IS_BROWSER, HOST, APP_NAME, IS_TESTING, GOOGLE_API_KEY, GOOGLE_MAP_ID and the stringifyEnvValues helper so the override behavior matches params.extraEnvs/params.env.packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx (1)
91-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
LineStringcoordinates in bounds fitting.
boundCoordsignoresgeojsons.linestrings, so a widget that only renders routes never reachesfitBoundsand opens at the default center instead of the geometry.Suggested fix
const boundCoords = React.useMemo<{ lat: number; lng: number }[]>(() => { const coords: { lat: number; lng: number }[] = []; for (const point of geojsons.points?.features || []) { coords.push({ lat: point.geometry.coordinates[1], lng: point.geometry.coordinates[0] }); } + for (const linestring of geojsons.linestrings?.features || []) { + for (const [lng, lat] of linestring.geometry.coordinates) { + coords.push({ lat, lng }); + } + } for (const polygon of geojsons.polygons?.features || []) { const { minLat, maxLat, minLong, maxLong } = polygon.properties; if (minLat !== undefined && maxLat !== undefined && minLong !== undefined && maxLong !== undefined) { coords.push({ lat: minLat, lng: minLong }); coords.push({ lat: maxLat, lng: maxLong });Also applies to: 110-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx` around lines 91 - 106, boundCoords currently only collects points and polygon corner coordinates, so LineString geometries in geojsons.linestrings are ignored and the map never calls fitBounds for routes; update the logic in the boundCoords useMemo to also iterate over geojsons.linestrings?.features and for each LineString feature push each coordinate as { lat: coord[1], lng: coord[0] } into coords, and make the same update for the other similar bounds collection block (the second boundCoords-like section) so LineString routes are included when computing fitBounds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx`:
- Around line 37-44: InfoMarker currently always renders AdvancedMarker which
fails when getGoogleMapId() is undefined; change InfoMarker to conditionally
render AdvancedMarker only when getGoogleMapId() returns a value and fall back
to the legacy Marker component otherwise (keep same props and icon handling).
Also update the bounds calculation that builds boundCoords (where boundCoords
currently includes geojsons.points and geojsons.polygons) to include coordinates
extracted from geojsons.linestrings (flatten polyline coordinates into the same
array) so fitBounds will include route-only widgets rendered as polylines.
In
`@packages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsx`:
- Around line 69-85: MapMarker currently always returns <AdvancedMarker>; change
it to check for the presence of the mapId (prop or context value you use for the
map) and render <AdvancedMarker> only when mapId is defined, otherwise render
the legacy marker component (e.g., <Marker> or the google.maps.Marker wrapper
you use). Ensure both branches pass the same semantic values: position built
from markerData.position, draggable from markerData.draggable, onDragEnd wired
to markerData.draggable ? onDragEnd : undefined, onClick to markerData.onClick,
and the icon (markerData.icon.url/size) so behavior matches the AdvancedMarker
fallback. Update MapMarker to reference AdvancedMarker and the legacy Marker
branch accordingly.
---
Outside diff comments:
In
`@packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx`:
- Around line 91-106: boundCoords currently only collects points and polygon
corner coordinates, so LineString geometries in geojsons.linestrings are ignored
and the map never calls fitBounds for routes; update the logic in the
boundCoords useMemo to also iterate over geojsons.linestrings?.features and for
each LineString feature push each coordinate as { lat: coord[1], lng: coord[0] }
into coords, and make the same update for the other similar bounds collection
block (the second boundCoords-like section) so LineString routes are included
when computing fitBounds.
In `@packages/evolution-frontend/src/utils/dev/webpackCommon.ts`:
- Around line 107-114: The build injects GOOGLE_MAP_ID from process.env but
should honor CommonWebpackConfigParams.env overrides; update the 'process.env'
mapping for GOOGLE_MAP_ID in webpackCommon.ts to read from params.env first
(e.g. use params.env?.GOOGLE_MAP_ID or equivalent) and fall back to
process.env.GOOGLE_MAP_ID, ensuring the value is JSON.stringify'd like the other
entries; reference the mapping block that includes IS_BROWSER, HOST, APP_NAME,
IS_TESTING, GOOGLE_API_KEY, GOOGLE_MAP_ID and the stringifyEnvValues helper so
the override behavior matches params.extraEnvs/params.env.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4cdfbcb4-5324-4370-bbf3-f4970d7587cb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
.env.exampleCHANGELOG.mdpackages/evolution-frontend/package.jsonpackages/evolution-frontend/src/components/inputs/__tests__/InputMapFindPlace.test.tsxpackages/evolution-frontend/src/components/inputs/__tests__/InputMapPoint.test.tsxpackages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsxpackages/evolution-frontend/src/config/googleMaps.config.tspackages/evolution-frontend/src/utils/dev/webpackCommon.tspackages/evolution-frontend/tsconfig.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: generate-and-build-generator (24.x)
- GitHub Check: pr-build-check
- GitHub Check: code-lint
- GitHub Check: build-and-test (24.x)
- GitHub Check: test-sequential (24.x)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-04-10T18:04:30.397Z
Learnt from: tahini
Repo: chairemobilite/evolution PR: 1491
File: packages/evolution-common/src/services/questionnaire/sections/visitedPlaces/widgetNextPlaceCategory.ts:25-31
Timestamp: 2026-04-10T18:04:30.397Z
Learning: In `packages/evolution-common/src/services/odSurvey/helpers.ts`, ensure `getHomeAddressOneLine` and the internal `getAddressOneLine` HTML-escape every address field (e.g., via lodash `_escape`) before assembling the returned one-line HTML string. Put the escaping in these helpers so all callers automatically receive a safe HTML string, and avoid requiring individual call sites to perform escaping.
Applied to files:
.env.example
📚 Learning: 2026-04-17T16:19:46.635Z
Learnt from: samuel-duhaime
Repo: chairemobilite/evolution PR: 1505
File: packages/evolution-generator/src/scripts/generate_choices.py:189-202
Timestamp: 2026-04-17T16:19:46.635Z
Learning: In packages/evolution-generator/src/scripts/generate_choices.py, when generating i18next translation keys for choices, do not escape dots in the choice value. Build translation keys using the literal format `choices:<choiceName>.<value>` (e.g., `choices:<choiceName>.some.value`) so that dot-containing values remain unmodified and match the existing i18next key convention. Do not add extra escaping logic just to handle dots.
Applied to files:
.env.example
📚 Learning: 2026-03-31T18:20:32.322Z
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1462
File: packages/evolution-common/src/services/baseObjects/Completable.ts:1-6
Timestamp: 2026-03-31T18:20:32.322Z
Learning: In this repository (chairemobilite/evolution), do not flag or require a copyright year in file headers. For new files, the header format `Copyright Polytechnique Montreal and contributors` (without a year) is acceptable. Only verify the header presence/consistency, not the presence of the year.
Applied to files:
packages/evolution-frontend/src/utils/dev/webpackCommon.tspackages/evolution-frontend/src/components/inputs/__tests__/InputMapFindPlace.test.tsxpackages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsxpackages/evolution-frontend/src/components/inputs/__tests__/InputMapPoint.test.tsxpackages/evolution-frontend/src/config/googleMaps.config.tspackages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsxpackages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsx
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 26-26: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
🔇 Additional comments (2)
packages/evolution-frontend/src/components/inputs/__tests__/InputMapFindPlace.test.tsx (1)
93-95: LGTM!Also applies to: 158-351
packages/evolution-frontend/src/components/inputs/__tests__/InputMapPoint.test.tsx (1)
9-9: LGTM!Also applies to: 30-36, 44-45, 58-58, 71-90, 107-178
| const InfoMarker: React.FC<{ | ||
| position: google.maps.LatLngLiteral; | ||
| icon?: { url: string; size: [number, number] }; | ||
| }> = ({ position, icon }) => ( | ||
| <AdvancedMarker position={position}> | ||
| {icon && <img src={icon.url} width={icon.size[0]} height={icon.size[1]} alt="" />} | ||
| </AdvancedMarker> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx"
echo "== File = $FILE"
ls -l "$FILE"
echo
echo "== Lines 1-120"
sed -n '1,120p' "$FILE" | cat -n
echo
echo "== Lines 120-200"
sed -n '120,200p' "$FILE" | cat -n
echo
echo "== Lines 200-320"
sed -n '200,320p' "$FILE" | cat -n
echo
echo "== Search for getGoogleMapId and boundCoords"
rg -n "getGoogleMapId|boundCoords|linestring|bounds" "$FILE"
echo
echo "== Snippet around InfoMarker definition"
rg -n "const InfoMarker" -n "$FILE"
sed -n '30,70p' "$FILE" | cat -nRepository: chairemobilite/evolution
Length of output: 18894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect getGoogleMapId implementation
rg -n "function getGoogleMapId|getGoogleMapId\s*=" -S packages/evolution-frontend/src/config || true
rg -n "getGoogleMapId" -S packages/evolution-frontend/src/config/googleMaps.config || true
ls -l packages/evolution-frontend/src/config/googleMaps.config || true
sed -n '1,200p' packages/evolution-frontend/src/config/googleMaps.config | cat -n || true
# 2) Find other Google map components for "no GOOGLE_MAP_ID" / <Marker> fallback patterns
rg -n "GOOGLE_MAP_ID|AdvancedMarker" packages/evolution-frontend/src/components/inputs/maps/google -S
rg -n "<Marker|\\bMarker\\b" packages/evolution-frontend/src/components/inputs/maps/google -S
# 3) Look for conditional marker rendering in the codebase
rg -n "mapId\\s*\\?|GOOGLE_MAP_ID.*fallback|AdvancedMarker.*mapId" packages/evolution-frontend/src -SRepository: chairemobilite/evolution
Length of output: 2188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect InputMapGoogle.tsx for marker fallback patterns
FILE="packages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsx"
ls -l "$FILE"
sed -n '1,220p' "$FILE" | cat -n
sed -n '220,520p' "$FILE" | cat -n
# Search for conditional rendering based on mapId
rg -n "getGoogleMapId\\(|mapId\\b|GOOGLE_MAP_ID|AdvancedMarker\\b|\\<Marker\\b|\\bMarker\\b" \
packages/evolution-frontend/src/components/inputs/maps/google -S
# Search for Marker import usage across whole evolution-frontend
rg -n "from '`@vis`\\.gl/react-google-maps'.*Marker|\\bMarker\\b\\s*," packages/evolution-frontend/src -SRepository: chairemobilite/evolution
Length of output: 14813
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect googleMaps.config.ts implementation
FILE="packages/evolution-frontend/src/config/googleMaps.config.ts"
ls -l "$FILE"
sed -n '1,120p' "$FILE" | cat -n
rg -n "getGoogleMapId" "$FILE" -n
# Search for Marker imports/usages in google maps inputs
rg -n "from '`@vis`\\.gl/react-google-maps'|\\bMarker\\b" packages/evolution-frontend/src/components/inputs/maps/google -S
# Search for any Marker usage anywhere in evolution-frontend
rg -n "\\bMarker\\b" packages/evolution-frontend/src/components/inputs/maps -SRepository: chairemobilite/evolution
Length of output: 3418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect googleMaps.config.ts implementation
FILE="packages/evolution-frontend/src/config/googleMaps.config.ts"
ls -l "$FILE"
sed -n '1,120p' "$FILE" | cat -n
rg -n "getGoogleMapId" "$FILE" -n
# Search for Marker imports/usages in google maps inputs
rg -n "from '`@vis`\\.gl/react-google-maps'|\\bMarker\\b" packages/evolution-frontend/src/components/inputs/maps/google -S
# Search for any Marker usage anywhere in evolution-frontend
rg -n "\\bMarker\\b" packages/evolution-frontend/src/components/inputs/maps -SRepository: chairemobilite/evolution
Length of output: 3418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\bMarker\b" packages/evolution-frontend/src/components/inputs/maps/google -S
rg -n "from '`@vis`\.gl/react-google-maps'[\s\S]*Marker" packages/evolution-frontend/src/components/inputs/maps/google -S || true
rg -n "import\s*{[^}]*\bMarker\b[^}]*}\s*from\s*'`@vis`\.gl/react-google-maps'" packages/evolution-frontend/src -S || trueRepository: chairemobilite/evolution
Length of output: 421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
set -x
# Look for "Marker" identifier usage in the Google maps inputs folder
rg -n "\bMarker\b" packages/evolution-frontend/src/components/inputs/maps/google -S
# Look for imports from vis-gl that include Marker
rg -n "from '`@vis`\.gl/react-google-maps'" packages/evolution-frontend/src/components/inputs/maps/google -S
rg -n "Marker" packages/evolution-frontend/src/components/inputs/maps/google -S
# Check overlays file(s) too
ls -1 packages/evolution-frontend/src/components/inputs/maps/google
rg -n "\bMarker\b" packages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsx -S || true
sed -n '1,220p' packages/evolution-frontend/src/components/inputs/maps/google/GoogleMapOverlays.tsx | cat -nRepository: chairemobilite/evolution
Length of output: 8059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx"
rg -n "<InfoMarker|boundCoords|linestrings|fitBounds|getGoogleMapId" "$FILE" -SRepository: chairemobilite/evolution
Length of output: 675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "from '`@vis`\.gl/react-google-maps'|from \"`@vis`\.gl/react-google-maps\"" -S packages/evolution-frontend/src
rg -n "Marker" packages/evolution-frontend/src -SRepository: chairemobilite/evolution
Length of output: 11596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/evolution-frontend/src/components/admin/validations/InterviewMap.tsx"
ls -l "$FILE"
sed -n '1,140p' "$FILE" | cat -n
sed -n '520,620p' "$FILE" | cat -nRepository: chairemobilite/evolution
Length of output: 12579
Add legacy marker fallback when GOOGLE_MAP_ID is missing (and fix bounds for route-only widgets)
InfoMarkeralways renders<AdvancedMarker>, butgetGoogleMapId()returnsundefinedwhenGOOGLE_MAP_IDisn’t set; this needs a fallback to the legacy<Marker>so point markers still render.boundCoordsincludes onlygeojsons.points+geojsons.polygons;geojsons.linestringsare rendered as polylines but excluded fromfitBounds, so route-only widgets may not zoom to the route.
Suggested marker fallback
import {
APIProvider,
AdvancedMarker,
Map,
+ Marker,
MapCameraChangedEvent,
useApiIsLoaded,
useMap
} from '`@vis.gl/react-google-maps`';
@@
const InfoMarker: React.FC<{
position: google.maps.LatLngLiteral;
icon?: { url: string; size: [number, number] };
-}> = ({ position, icon }) => (
- <AdvancedMarker position={position}>
- {icon && <img src={icon.url} width={icon.size[0]} height={icon.size[1]} alt="" />}
- </AdvancedMarker>
-);
+ mapId?: string;
+}> = ({ position, icon, mapId }) =>
+ mapId ? (
+ <AdvancedMarker position={position}>
+ {icon && <img src={icon.url} width={icon.size[0]} height={icon.size[1]} alt="" />}
+ </AdvancedMarker>
+ ) : (
+ <Marker
+ position={position}
+ icon={
+ icon
+ ? {
+ url: icon.url,
+ scaledSize: new google.maps.Size(icon.size[0], icon.size[1])
+ }
+ : undefined
+ }
+ />
+ );
@@
- const title =
+ const mapId = getGoogleMapId();
+ const title =
surveyHelper.translateString(props.widgetConfig.title, i18n, props.interview, props.path, props.user) || '';
@@
<InfoMarker
key={`gMarker_infoMap_${props.path}__${i}`}
position={{ lat: point.geometry.coordinates[1], lng: point.geometry.coordinates[0] }}
icon={point.properties.icon}
+ mapId={mapId}
/>
);
}
@@
- const mapId = getGoogleMapId();
-
return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/evolution-frontend/src/components/inputs/maps/google/InfoMapGoogle.tsx`
around lines 37 - 44, InfoMarker currently always renders AdvancedMarker which
fails when getGoogleMapId() is undefined; change InfoMarker to conditionally
render AdvancedMarker only when getGoogleMapId() returns a value and fall back
to the legacy Marker component otherwise (keep same props and icon handling).
Also update the bounds calculation that builds boundCoords (where boundCoords
currently includes geojsons.points and geojsons.polygons) to include coordinates
extracted from geojsons.linestrings (flatten polyline coordinates into the same
array) so fitBounds will include route-only widgets rendered as polylines.
There was a problem hiding this comment.
legacy removed completely, easier.
There was a problem hiding this comment.
@kaligrafy Got it — dropping legacy Marker entirely and requiring GOOGLE_MAP_ID is the cleaner approach. 👍
One remaining point from the original comment: boundCoords still excludes geojsons.linestrings, so fitBounds won't zoom to routes on route-only widgets. Would you like to open a follow-up issue for that, or address it in this PR?
🧠 Learnings used
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1462
File: packages/evolution-common/src/services/baseObjects/Completable.ts:1-6
Timestamp: 2026-03-31T18:20:32.322Z
Learning: In this repository (chairemobilite/evolution), do not flag or require a copyright year in file headers. For new files, the header format `Copyright Polytechnique Montreal and contributors` (without a year) is acceptable. Only verify the header presence/consistency, not the presence of the year.
| const MapMarker: React.FunctionComponent<{ | ||
| markerData: MarkerData; | ||
| onDragEnd?: (e: google.maps.MapMouseEvent) => void; | ||
| }> = ({ markerData, onDragEnd }) => { | ||
| const position = { | ||
| lat: markerData.position.geometry.coordinates[1], | ||
| lng: markerData.position.geometry.coordinates[0] | ||
| }; | ||
| return ( | ||
| <AdvancedMarker | ||
| position={position} | ||
| draggable={markerData.draggable} | ||
| onDragEnd={markerData.draggable ? onDragEnd : undefined} | ||
| onClick={markerData.onClick} | ||
| > | ||
| <img src={markerData.icon.url} width={markerData.icon.size[0]} height={markerData.icon.size[1]} alt="" /> | ||
| </AdvancedMarker> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Google Maps AdvancedMarkerElement require a Map ID, and what is the recommended fallback in @vis.gl/react-google-maps when mapId is undefined?
💡 Result:
Yes. Google requires a mapId to use AdvancedMarkerElement: “A map ID is required to use Advanced Markers… Set a map ID in map options” [1][2]. If the mapId is missing, Advanced Markers cannot load: “Advanced markers requires a map ID. If the map ID is missing, advanced markers cannot load.” [2]. In @vis.gl/react-google-maps, the component is “only… on maps using cloud-based map styling (i.e. the Map-component has a mapId specified)” [3]. So when mapId is undefined, the recommended fallback is to use a plain/raster map instead: the component docs state that if mapId is not set, the cloud config will fall back to raster as needed (“vector maps may not be available… and the map will fall back to a raster map as needed”) [4], but you should not expect to work without mapId [3]. Practical implication: if you need AdvancedMarker, pass mapId (e.g. DEMO_MAP_ID for testing) to the . Otherwise, render markers using something that does not depend on AdvancedMarker/cloud styling.
Citations:
- 1: https://developers.google.com/maps/documentation/javascript/advanced-markers/add-marker
- 2: https://developers.google.com/maps/documentation/javascript/advanced-markers/start
- 3: https://visgl.github.io/react-google-maps/docs/api-reference/components/advanced-marker
- 4: https://visgl.github.io/react-google-maps/docs/api-reference/components/map
Restore legacy marker fallback when GOOGLE_MAP_ID/mapId is missing
Google Advanced Markers require a mapId, and @vis.gl/react-google-maps notes <AdvancedMarker> only works on maps with a mapId. MapMarker currently renders <AdvancedMarker> unconditionally (lines 69-85), so the promised no-GOOGLE_MAP_ID fallback isn’t implemented—add conditional rendering to use the legacy marker when mapId is undefined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/evolution-frontend/src/components/inputs/maps/google/InputMapGoogle.tsx`
around lines 69 - 85, MapMarker currently always returns <AdvancedMarker>;
change it to check for the presence of the mapId (prop or context value you use
for the map) and render <AdvancedMarker> only when mapId is defined, otherwise
render the legacy marker component (e.g., <Marker> or the google.maps.Marker
wrapper you use). Ensure both branches pass the same semantic values: position
built from markerData.position, draggable from markerData.draggable, onDragEnd
wired to markerData.draggable ? onDragEnd : undefined, onClick to
markerData.onClick, and the icon (markerData.icon.url/size) so behavior matches
the AdvancedMarker fallback. Update MapMarker to reference AdvancedMarker and
the legacy Marker branch accordingly.
There was a problem hiding this comment.
should be fixed by removing old method/markers
There was a problem hiding this comment.
@kaligrafy Thanks for the clarification. Just to confirm the intent: by removing old markers entirely and always using <AdvancedMarker>, GOOGLE_MAP_ID becomes effectively required for markers to render. Deployments that don't set it will silently get no markers (or a Google Maps error), rather than falling back to raster <Marker>.
If that's the intentional breaking change (and it's documented in the CHANGELOG), that's fine — just worth making sure .env.example and the CHANGELOG clearly state that GOOGLE_MAP_ID is required (not optional) for markers to work, not just needed for vector maps.
🧠 Learnings used
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1462
File: packages/evolution-common/src/services/baseObjects/Completable.ts:1-6
Timestamp: 2026-03-31T18:20:32.322Z
Learning: In this repository (chairemobilite/evolution), do not flag or require a copyright year in file headers. For new files, the header format `Copyright Polytechnique Montreal and contributors` (without a year) is acceptable. Only verify the header presence/consistency, not the presence of the year.
2 participants
Summary
Replaces
@react-google-maps/apiwith@vis.gl/react-google-mapsand rewrites the two Google map widgets (InputMapGoogle,InfoMapGoogle) on top of<APIProvider>/<Map>and theuseMap()/useApiIsLoaded()hooks.Addresses #453 (replace deprecated package). Sets the foundation for #1146 (replace deprecated
PlacesService— separate follow-up) and #1156 (missing photos — same follow-up).What changed
@react-google-maps/apiremoved,@vis.gl/react-google-maps(+ transitive@types/google.maps) added.googleMaps.config.ts: now exposes theAPIProviderconfig (apiKey/libraries/region/language) and agetGoogleMapId()helper that readsprocess.env.GOOGLE_MAP_ID.<Map>runs uncontrolled (defaultCenter/defaultZoom); programmatic recentering goes throughmap.panTo(). This fixes the freezing-pan bug we hit when<Map>was given a controlledcenterprop.<AdvancedMarker>when a Google Map ID is configured (vector maps), otherwise fall back to the legacy<Marker>so existing deployments without a Map ID keep working.Polyline/Polygonare not provided by@vis.gl/react-google-maps, soInfoMaprenders them via small imperative wrappers (MapPolyline/MapPolygoninGoogleMapOverlays.tsx) mounted on the current map viauseMap().withTranslationHOC →useTranslationhook in both widgets.webpackCommon.tsforwardsGOOGLE_MAP_IDintoprocess.env.tsconfig.json: includesgoogle.mapsintypes(the global namespace was previously pulled in transitively by the old package)..env.example: documents the optionalGOOGLE_MAP_IDwith links to the official Google docs (Map ID + AdvancedMarker migration).Tests
Refactored existing geocoding tests to use parameterized
test.eachand a sharedrenderWidgethelper (less duplication, easier to debug). Added one missing case inInputMapFindPlace:This covers the legal-compliance path (place_id preservation under Google ToS / OIQ data-retention requirements) for the multi-select branch, which was previously only covered for the single-result auto-select branch.
All map tests pass: 13/13 (vs 17 before; coverage equal-or-better thanks to parameterization).
Coded by Claude Opus 4.7, reviewed and tested by @kaligrafy
Out of scope (follow-ups)
#1146deprecation ofgoogle.maps.places.PlacesService→ replace withgoogle.maps.places.Place.searchByTextinGoogleGeocoder.tsx.#1156missing photos → pre-compute photo URLs in the geocoder and tightenplaceData.photostostring[].GoogleGeocoderparameterized unit tests.#888OSM/MapLibre provider — unblocked by the existingMapProviderAdapter.Test plan
yarn compileinevolution-frontend).yarn test --testPathPatterns="InputMap|InfoMap").GOOGLE_MAP_IDset).GOOGLE_MAP_IDstill render maps via the legacy<Marker>fallback.Summary by CodeRabbit
New Features
Behavior Changes
Tests