Skip to content

fix: prevent memory leak in ScriptGoogleMapsAdvancedMarkerElement - #649

Merged
harlan-zw merged 1 commit into
mainfrom
worktree-fix-marker-memory-leak
Mar 18, 2026
Merged

fix: prevent memory leak in ScriptGoogleMapsAdvancedMarkerElement#649
harlan-zw merged 1 commit into
mainfrom
worktree-fix-marker-memory-leak

Conversation

@harlan-zw

@harlan-zwharlan-zw commented Mar 18, 2026

Copy link
Copy Markdown
Collaborator

🔗 Linked issue

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • [x 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Track unmounted state so async marker creation is aborted if the component unmounts before importLibrary('marker') resolves. Previously the marker would be created and attached to the map with no owner to clean it up.

Resolves#646

Track unmounted state so async marker creation is aborted if the
component unmounts before `importLibrary('marker')` resolves.
Previously the marker would be created and attached to the map
with no owner to clean it up.
Resolves#646
@vercel

vercelBot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
scripts-playgroundReadyReadyPreview, CommentMar 18, 2026 3:35pm

@pkg-pr-new

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@nuxt/scripts@649

commit: 09f73b3

@coderabbitai

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This change addresses a memory leak in the ScriptGoogleMapsAdvancedMarkerElement component. A ref-based unmount flag (isUnmounted) was introduced to track when the component is unmounted. During the async library import process, the component checks this flag before creating the AdvancedMarkerElement. If unmount occurred during the async operation, the element creation is aborted, preventing detached markers from remaining on the map after the component is destroyed. The setup flow was also extended to support marker clusterer integration and reactive options updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly and concisely describes the main change: preventing a memory leak in the ScriptGoogleMapsAdvancedMarkerElement component. It follows conventional commits format (fix prefix) and directly relates to the code changes.
Description check✅ PassedThe description is mostly complete with the bug fix type selected and a clear explanation of what the change does and which issue it resolves. The only minor issue is a missing closing bracket in one checkbox.
Linked Issues check✅ PassedThe code changes fully address the requirements from issue #646: tracking unmounted state (isUnmounted ref), aborting marker creation if unmounted during async import, and preventing markers from being created and attached after component unmount.
Out of Scope Changes check✅ PassedAll code changes are directly related to fixing the memory leak in ScriptGoogleMapsAdvancedMarkerElement, with lifecycle-aware handling to prevent detached markers after unmount. No out-of-scope changes detected.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-fix-marker-memory-leak
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/runtime/components/GoogleMaps/ScriptGoogleMapsAdvancedMarkerElement.vue (1)

87-100: ⚠️ Potential issue | 🟠 Major

Align unmount condition with mount to prevent missed detachment on early unmounts.

The mount path checks markerClustererContext?.markerClusterer.value (both context AND initialized value), but unmount only checks markerClustererContext (context alone). If a marker mounts while the clusterer context exists but its .value is still undefined (async initialization window), mount uses the else branch to set map = mapContext.map.value. However, on unmount, the condition enters the if branch, attempts optional removeMarker, and skips the else that would clear map = null. This leaves the marker attached to the map, undoing the leak fix.

Proposed fix
- if (markerClustererContext) {+ if (markerClustererContext?.markerClusterer.value) {
markerClustererContext.markerClusterer.value?.removeMarker(advancedMarkerElement.value)
}
else {
advancedMarkerElement.value.map = null
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/components/GoogleMaps/ScriptGoogleMapsAdvancedMarkerElement.vue`
around lines 87 - 100, The unmount branch uses markerClustererContext alone
which can cause skipping the map-clearing path if markerClustererContext exists
but markerClustererContext.markerClusterer.value is still undefined; update the
unmount condition to match the mount check (use
markerClustererContext?.markerClusterer.value) so you call
markerClustererContext.markerClusterer.value?.removeMarker(advancedMarkerElement.value)
only when the clusterer instance exists and otherwise always run
advancedMarkerElement.value.map = null after clearing listeners (reference
isUnmounted, advancedMarkerElement,
mapContext.mapsApi.value.event.clearInstanceListeners,
markerClustererContext?.markerClusterer.value, and
advancedMarkerElement.value.map = null).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/runtime/components/GoogleMaps/ScriptGoogleMapsAdvancedMarkerElement.vue`:
- Around line 87-100: The unmount branch uses markerClustererContext alone which
can cause skipping the map-clearing path if markerClustererContext exists but
markerClustererContext.markerClusterer.value is still undefined; update the
unmount condition to match the mount check (use
markerClustererContext?.markerClusterer.value) so you call
markerClustererContext.markerClusterer.value?.removeMarker(advancedMarkerElement.value)
only when the clusterer instance exists and otherwise always run
advancedMarkerElement.value.map = null after clearing listeners (reference
isUnmounted, advancedMarkerElement,
mapContext.mapsApi.value.event.clearInstanceListeners,
markerClustererContext?.markerClusterer.value, and
advancedMarkerElement.value.map = null).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 641bbd47-76f1-4fce-9161-df2f76340fef

📥 Commits

Reviewing files that changed from the base of the PR and between a6401a4 and 09f73b3.

📒 Files selected for processing (1)
  • src/runtime/components/GoogleMaps/ScriptGoogleMapsAdvancedMarkerElement.vue

@harlan-zw
harlan-zw merged commit a597de9 into mainMar 18, 2026
10 checks passed
@harlan-zw
harlan-zw deleted the worktree-fix-marker-memory-leak branch March 18, 2026 15:41
harlan-zw added a commit that referenced this pull request Mar 19, 2026
The previous fix (#649) only handled one race condition — unmounting during
`await importLibrary`. The actual leak was caused by orphaned Vue watchers:
watchers created after an `await` lose component instance context and are
never auto-stopped on unmount, retaining the entire reactive scope.
- Create `useGoogleMapsResource` composable encoding lifecycle safety
- Move all options watchers to synchronous setup scope (Vue auto-stops)
- Add unmount guards to all async resource creation callbacks
- Null all resource refs on unmount to release Google Maps objects
- Convert `let` variables to `shallowRef` for proper GC
- Make parent `onBeforeUnmount` synchronous (Vue doesn't await async hooks)
- Clear `libraries` and `queryToLatLngCache` on parent unmount
- Extract injection keys to shared `injectionKeys.ts`
Closes#646
@harlan-zwharlan-zw mentioned this pull request Apr 15, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cleanup causing a memory leak with <ScriptGoogleMapsAdvancedMarkerElement>

1 participant

@harlan-zw