Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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.
*
Expand Down
111 changes: 110 additions & 1 deletion test/unit/google-maps-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
})
Loading