diff --git a/packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue b/packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue index 8a37d4ef..d0db9b3d 100644 --- a/packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue +++ b/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(), { // @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((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((resolve, reject) => { diff --git a/packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource.ts b/packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource.ts index 081df7d2..3a302998 100644 --- a/packages/script/src/runtime/components/GoogleMaps/useGoogleMapsResource.ts +++ b/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 @@ -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 + map: ShallowRef + status: Ref + load: () => Promise | unknown +}): Promise { + 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((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. * diff --git a/test/unit/google-maps-lifecycle.test.ts b/test/unit/google-maps-lifecycle.test.ts index 48fd5533..dfbd93ed 100644 --- a/test/unit/google-maps-lifecycle.test.ts +++ b/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({ places: {} }) + const map = shallowRef({}) + const status = ref('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(undefined) + const map = shallowRef(undefined) + const status = ref('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(undefined) + const map = shallowRef(undefined) + const status = ref('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(undefined) + const map = shallowRef(undefined) + const status = ref('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(undefined) + const map = shallowRef(undefined) + const status = ref('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({ places: {} }) + const map = shallowRef(undefined) + const status = ref('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) + }) +})