fix(google-maps): handle marker + mapOptions.styles conflict#719
fix(google-maps): handle marker + mapOptions.styles conflict#719
Conversation
Closes #716 JSON `mapOptions.styles` previously dropped `mapId`, breaking `AdvancedMarkerElement` which v1 `ScriptGoogleMapsMarker` requires. Always pass `mapId` through; add a dev warning when `styles` is set pointing users to cloud-based styling.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated documentation to change JSON styles wording to “Styles apply” and add an amber callout stating JSON Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/scripts/google-maps/1.guides/2.map-styling.md`:
- Line 15: Update the docs sentence that says "Styles apply to the static map
placeholder and to the interactive map when the map has no markers" to reflect
current runtime behavior: because ScriptGoogleMaps.vue always passes a mapId
(fallback 'map') in the options computation (see options computation in
ScriptGoogleMaps.vue around the current lines 225–235), Google Maps ignores JSON
`styles` for the interactive map and JSON styles only affect the static
placeholder in v1; alternatively note that restoring the original behavior would
require adopting the scoped-fallback change in the component's options
computation.
In `@packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue`:
- Around line 225-235: The computed options currently forces mapId via const
mapId = currentMapId.value || 'map', which breaks JSON styles and silently
breaks AdvancedMarkerElement; change this to not default to the literal 'map'
(use const FALLBACK_MAP_ID = 'map' only if you keep it, otherwise remove it) and
instead set mapId = currentMapId.value ?? undefined; then add validation in the
ScriptGoogleMapsMarker (or wherever markers are registered) to throw or log a
clear error if a marker component is mounted but no valid/currentMapId is
provided (fail-fast), or alternatively only apply the fallback when no markers
are present (check a hasMarkers flag/registry before using FALLBACK_MAP_ID);
also update handling around options (the computed options function and its defu
call, and any docs/examples referencing mapOptions.styles) to warn when mapId is
present that styles will be ignored and document the breaking change if markers
are mandatory.
🪄 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: CHILL
Plan: Pro
Run ID: a6647212-e708-4754-8fa7-4a9cf2f939b4
📒 Files selected for processing (2)
docs/content/scripts/google-maps/1.guides/2.map-styling.mdpackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue
Preserve JSON styles for maps without markers (no regression for styles-only users). Warn at the marker, where the conflict actually surfaces, when the parent map has no mapId. Update the styling guide to state the conflict is between styles and markers.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsMarker.vue (1)
60-62: LGTM — dev warning correctly targets the styles/mapId conflict.The
!map.get('mapId')check ongoogle.maps.Map(anMVCObject) is a valid way to detect the styled-map case, and it aligns with the parent'soptionscomputed wheremapIdis dropped only whenmapOptions.stylesis set. Gating onimport.meta.devkeeps this out of production bundles.One optional consideration: if a user mounts many
<ScriptGoogleMapsMarker>children against the same styled map, this will log once per marker. If that becomes noisy in practice, you could dedupe by tagging the map (e.g.map.set('__nuxtScriptsStylesWarned', true)) after the first warn. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsMarker.vue` around lines 60 - 62, The dev-only console warning in ScriptGoogleMapsMarker.vue currently logs once per marker when a styled map lacks a mapId; to avoid noisy repeat warnings, modify the logic around the existing import.meta.dev && !map.get('mapId') check to first test a custom flag on the map (e.g. map.get('__nuxtScriptsStylesWarned')) and only call console.warn and then set that flag (map.set('__nuxtScriptsStylesWarned', true')) on first occurrence; keep the existing guard and message text intact so behavior and wording remain unchanged except for deduplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsMarker.vue`:
- Around line 60-62: The dev-only console warning in ScriptGoogleMapsMarker.vue
currently logs once per marker when a styled map lacks a mapId; to avoid noisy
repeat warnings, modify the logic around the existing import.meta.dev &&
!map.get('mapId') check to first test a custom flag on the map (e.g.
map.get('__nuxtScriptsStylesWarned')) and only call console.warn and then set
that flag (map.set('__nuxtScriptsStylesWarned', true')) on first occurrence;
keep the existing guard and message text intact so behavior and wording remain
unchanged except for deduplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa373025-7fc0-4a82-9cd4-c1603f9a441c
📒 Files selected for processing (3)
docs/content/scripts/google-maps/1.guides/2.map-styling.mdpackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vuepackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsMarker.vue
✅ Files skipped from review due to trivial changes (1)
- packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/content/scripts/google-maps/1.guides/2.map-styling.md
Google's pre-registered dev map ID. An arbitrary string like 'map' triggers a console warning every load; DEMO_MAP_ID renders silently and matches Google's documented convention for examples.
harlan-zw
changed the title
1 participant
🔗 Linked issue
Resolves #716
❓ Type of change
📚 Description
v1
ScriptGoogleMapsMarkerusesAdvancedMarkerElement, which requires amapId. Google Maps treats JSONstylesandmapIdas mutually exclusive, so combiningmapOptions.styleswith a marker silently broke the map ("This Page Can't Load Google Maps Correctly.").Google's API makes this genuinely unresolvable (styles need the raster renderer, advanced markers need the vector renderer). Rather than pick a side, this PR:
ScriptGoogleMaps(no regression for users with styled maps and no markers).ScriptGoogleMapsMarkerwhen the parent map has nomapId, pointing users to cloud-based map styling.mapIdfallback from the arbitrary'map'to Google's pre-registered'DEMO_MAP_ID', which silences the "initialized without a valid Map ID" warning Google logs for unregistered IDs.No runtime behavior change for existing users; only new behavior is a dev-mode warning at the point the conflict actually surfaces.