fix(google-maps): close races in resolveQueryToLatLng (#693)

Author: harlan-zw

packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue

@@ -138,7 +138,7 @@ import { defu } from 'defu'
 import { tryUseNuxtApp, useHead, useRuntimeConfig } from 'nuxt/app'
 import { computed, onBeforeUnmount, onMounted, provide, ref, shallowRef, toRaw, useAttrs, useTemplateRef, watch } from 'vue'
 import ScriptAriaLoadingIndicator from '../ScriptAriaLoadingIndicator.vue'
-import { MAP_INJECTION_KEY } from './useGoogleMapsResource'
+import { MAP_INJECTION_KEY, waitForMapsReady } from './useGoogleMapsResource'
 
 const props = withDefaults(defineProps<ScriptGoogleMapsProps>(), {
   // @ts-expect-error untyped
@@ -241,17 +241,11 @@ async function resolveQueryToLatLng(query: string) {
     throw new Error(`No location found for ${query}`)
   }
 
-  // Fallback: use Places API client-side
-  if (!mapsApi.value) {
-    await load()
-    // await new promise, watch until mapsApi is set
-    await new Promise<void>((resolve) => {
-      const _ = watch(mapsApi, () => {
-        _()
-        resolve()
-      })
-    })
-  }
+  // Fallback: use Places API client-side. Wait for both the maps API and a
+  // Map instance: resolveQueryToLatLng is publicly exposed and may be called
+  // before onLoaded has populated map.value, so constructing PlacesService
+  // without map would throw.
+  await waitForMapsReady({ mapsApi, map, status, load })
 
   const placesService = new mapsApi.value!.places.PlacesService(map.value!)
   const result = await new Promise<google.maps.LatLng>((resolve, reject) => {

packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource.ts

@@ -1,6 +1,6 @@
 import type { InjectionKey, Ref, ShallowRef } from 'vue'
 import { whenever } from '@vueuse/core'
-import { inject, onUnmounted, ref, shallowRef } from 'vue'
+import { effectScope, inject, onUnmounted, ref, shallowRef, watch } from 'vue'
 
 export const MAP_INJECTION_KEY = Symbol('map') as InjectionKey<{
   map: ShallowRef<google.maps.Map | undefined>
@@ -41,6 +41,67 @@ export interface GoogleMapsResourceContext {
   mapsApi: typeof google.maps
 }
 
+/**
+ * Wait until the Google Maps API and a Map instance are both available.
+ *
+ * Triggers script loading via `load()` if not already loaded. Uses an
+ * immediate watcher (matching `importLibrary`'s pattern) to avoid the race
+ * where `load()` resolves synchronously: a non-immediate watcher would miss
+ * the change and the promise would hang forever.
+ *
+ * Rejects if `status` enters an `'error'` state before both refs are populated.
+ * Runs the watcher inside a detached effect scope so it is safe to call from
+ * any context (component setup, exposed methods, tests).
+ */
+export async function waitForMapsReady({
+  mapsApi,
+  map,
+  status,
+  load,
+}: {
+  mapsApi: ShallowRef<typeof google.maps | undefined>
+  map: ShallowRef<google.maps.Map | undefined>
+  status: Ref<string>
+  load: () => Promise<unknown> | unknown
+}): Promise<void> {
+  if (mapsApi.value && map.value)
+    return
+  if (status.value === 'error')
+    throw new Error('Google Maps script failed to load')
+
+  await load()
+
+  // load() may have populated both refs synchronously — re-check before
+  // installing a watcher to avoid the race that hangs the promise forever.
+  if (mapsApi.value && map.value)
+    return
+  if (status.value === 'error')
+    throw new Error('Google Maps script failed to load')
+
+  const scope = effectScope(true)
+  try {
+    await new Promise<void>((resolve, reject) => {
+      scope.run(() => {
+        watch(
+          [mapsApi, map, status],
+          ([api, m, s]) => {
+            if (api && m) {
+              resolve()
+              return
+            }
+            if (s === 'error')
+              reject(new Error('Google Maps script failed to load'))
+          },
+          { immediate: true },
+        )
+      })
+    })
+  }
+  finally {
+    scope.stop()
+  }
+}
+
 /**
  * Composable for safely managing Google Maps resource lifecycle.
  *

test/unit/google-maps-lifecycle.test.ts

@@ -4,7 +4,7 @@
 import { mount } from '@vue/test-utils'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { defineComponent, h, nextTick, onUnmounted, provide, ref, shallowRef } from 'vue'
-import { MAP_INJECTION_KEY, useGoogleMapsResource } from '../../packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource'
+import { MAP_INJECTION_KEY, useGoogleMapsResource, waitForMapsReady } from '../../packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource'
 import { createMockGoogleMapsAPI } from './__mocks__/google-maps-api'
 
 // Helper to create a wrapper component that provides mock map context
@@ -367,3 +367,112 @@ describe('google Maps component lifecycle - memory leak prevention', () => {
     )
   })
 })
+
+describe('waitForMapsReady', () => {
+  // Regression: the previous resolveQueryToLatLng wait pattern used a
+  // non-immediate watch after `await load()`. If load() populated mapsApi
+  // synchronously, the watcher missed the change and the promise hung
+  // forever. The fix re-checks state after load() and uses an immediate
+  // watcher (matching importLibrary's pattern). It also waits for `map`
+  // to be set, since PlacesService construction requires it.
+
+  it('returns immediately when both refs are already populated', async () => {
+    const mapsApi = shallowRef<any>({ places: {} })
+    const map = shallowRef<any>({})
+    const status = ref<string>('loaded')
+    const load = vi.fn(() => Promise.resolve())
+
+    await waitForMapsReady({ mapsApi, map, status, load })
+
+    // Should not have called load() at all
+    expect(load).not.toHaveBeenCalled()
+  })
+
+  it('does not hang when load() resolves synchronously', async () => {
+    // Reproduces the original race: load() populates mapsApi synchronously,
+    // so a non-immediate watcher would never fire.
+    const mapsApi = shallowRef<any>(undefined)
+    const map = shallowRef<any>(undefined)
+    const status = ref<string>('loading')
+    const load = vi.fn(() => {
+      // Synchronously populate both refs before returning the resolved promise
+      mapsApi.value = { places: {} }
+      map.value = {}
+      return Promise.resolve()
+    })
+
+    // The original race would hang forever — vitest's per-test timeout
+    // is the deterministic backstop, no wall-clock setTimeout needed.
+    await expect(waitForMapsReady({ mapsApi, map, status, load })).resolves.toBeUndefined()
+
+    expect(load).toHaveBeenCalledOnce()
+  })
+
+  it('resolves when refs are populated asynchronously after load()', async () => {
+    const mapsApi = shallowRef<any>(undefined)
+    const map = shallowRef<any>(undefined)
+    const status = ref<string>('loading')
+    const load = vi.fn(() => Promise.resolve())
+
+    const promise = waitForMapsReady({ mapsApi, map, status, load })
+
+    // Populate refs after a tick — simulates the normal onLoaded flow
+    await nextTick()
+    mapsApi.value = { places: {} }
+    await nextTick()
+    map.value = {}
+
+    await expect(promise).resolves.toBeUndefined()
+  })
+
+  it('rejects synchronously when status is already error', async () => {
+    const mapsApi = shallowRef<any>(undefined)
+    const map = shallowRef<any>(undefined)
+    const status = ref<string>('error')
+    const load = vi.fn(() => Promise.resolve())
+
+    await expect(
+      waitForMapsReady({ mapsApi, map, status, load }),
+    ).rejects.toThrow('Google Maps script failed to load')
+
+    // Should bail before calling load()
+    expect(load).not.toHaveBeenCalled()
+  })
+
+  it('rejects when status transitions to error during the wait', async () => {
+    const mapsApi = shallowRef<any>(undefined)
+    const map = shallowRef<any>(undefined)
+    const status = ref<string>('loading')
+    const load = vi.fn(() => Promise.resolve())
+
+    const promise = waitForMapsReady({ mapsApi, map, status, load })
+
+    await nextTick()
+    status.value = 'error'
+
+    await expect(promise).rejects.toThrow('Google Maps script failed to load')
+  })
+
+  it('waits for map even when mapsApi is set first', async () => {
+    // Guards against the second bug: PlacesService construction needs
+    // map.value, not just mapsApi.value.
+    const mapsApi = shallowRef<any>({ places: {} })
+    const map = shallowRef<any>(undefined)
+    const status = ref<string>('loading')
+    const load = vi.fn(() => Promise.resolve())
+
+    let settled = false
+    const promise = waitForMapsReady({ mapsApi, map, status, load }).then(() => {
+      settled = true
+    })
+
+    // Flush microtasks: settlement should not happen while map is undefined
+    await nextTick()
+    await nextTick()
+    expect(settled).toBe(false)
+
+    map.value = {}
+    await expect(promise).resolves.toBeUndefined()
+    expect(settled).toBe(true)
+  })
+})