Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add shouldCacheResult option #96

Merged
merged 4 commits into from
Nov 3, 2024
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
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
"lint-staged": "^15.2.10",
"prettier": "^3.3.3",
"prettier-plugin-packagejson": "^2.5.3",
"turbo": "^2.2.0",
"vitest": "^2.1.3"
"turbo": "^2.2.3",
"vitest": "^2.1.4"
},
"packageManager": "pnpm@9.6.0",
"pnpm": {
Expand Down
7 changes: 6 additions & 1 deletion package/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,12 @@ const page = await context.sanity.loadQuery<HomePage>(query, params, {
// display in the subrequest profiler, you can pass that here:
// debug: {
// displayName: 'query Homepage'
// }
// },

// You can also pass a function do determine whether or not to cache the response
// shouldCacheResult(value){
// return true
// },
},

// ...as well as other request options
Expand Down
6 changes: 3 additions & 3 deletions package/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@
"groq": "^3.62.3"
},
"devDependencies": {
"@sanity/pkg-utils": "^6.11.7",
"@sanity/pkg-utils": "^6.11.8",
"@sanity/plugin-kit": "^4.0.18",
"@sanity/semantic-release-preset": "^4.1.8",
"@shopify/hydrogen": "~2024.7.9",
"@shopify/hydrogen": "~2024.10.0",
"@shopify/remix-oxygen": "^2.0.9",
"@types/react": "^18.3.12",
"@typescript-eslint/eslint-plugin": "^7.18.0",
Expand All @@ -113,7 +113,7 @@
},
"peerDependencies": {
"@sanity/client": "^6.18.0",
"@shopify/hydrogen": "^2023.7.0 || ~2024.1.0 || ~2024.4.0 || ~2024.7.0",
"@shopify/hydrogen": "^2023.7.0 || ~2024.1.0 || ~2024.4.0 || ~2024.7.0 || ~2024.10.0",
"@shopify/remix-oxygen": "^1.0.0 || ^2.0.0",
"groq": "^3.41.0",
"react": "^18.0.0",
Expand Down
30 changes: 21 additions & 9 deletions package/src/context.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {QueryStore} from '@sanity/react-loader'
import {CacheShort, WithCache} from '@shopify/hydrogen'
import {CacheShort, type WithCache} from '@shopify/hydrogen'
import groq from 'groq'
import {beforeEach, describe, expect, it, vi} from 'vitest'

Expand All @@ -25,9 +25,11 @@ let withCache = vi.hoisted<WithCache | null>(() => null)

vi.mock('@shopify/hydrogen', async (importOriginal) => {
const module = await importOriginal<typeof import('@shopify/hydrogen')>()
withCache = vi
.fn()
.mockImplementation(module.createWithCache({cache, waitUntil: () => Promise.resolve()}))
withCache = module.createWithCache({
cache,
waitUntil: () => Promise.resolve(),
request: new Request('https://example.com'),
})

return {
...module,
Expand All @@ -44,6 +46,8 @@ vi.mock('@sanity/react-loader', async (importOriginal) => {
}
})

const runWithCache = vi.spyOn(withCache!, 'run')

const client = createClient({projectId: 'my-project-id', dataset: 'my-dataset'})

const query = groq`true`
Expand Down Expand Up @@ -73,17 +77,25 @@ describe('the Sanity request context', () => {
})

await contextWithDefaultStrategy.loadQuery<boolean>(query, params)
expect(withCache).toHaveBeenCalledWith(hashedQuery, defaultStrategy, expect.any(Function))
expect(cache.put).toHaveBeenCalled()
expect(runWithCache).toHaveBeenNthCalledWith(
1,
expect.objectContaining({cacheKey: hashedQuery, cacheStrategy: defaultStrategy}),
expect.any(Function),
)
expect(cache.put).toHaveBeenCalledOnce()
})

it('queries should use the cache strategy passed in `loadQuery`', async () => {
const strategy = CacheShort()
await sanityContext.loadQuery<boolean>(query, params, {
hydrogen: {cache: strategy},
})
expect(withCache).toHaveBeenCalledWith(hashedQuery, strategy, expect.any(Function))
expect(cache.put).toHaveBeenCalled()
expect(runWithCache).toHaveBeenNthCalledWith(
1,
expect.objectContaining({cacheKey: hashedQuery, cacheStrategy: strategy}),
expect.any(Function),
)
expect(cache.put).toHaveBeenCalledOnce()
})
})

Expand Down Expand Up @@ -122,6 +134,6 @@ describe('when configured for preview', () => {
it(`shouldn't cache queries`, async () => {
await previewContext.loadQuery<boolean>(query)
expect(loadQuery).toHaveBeenCalledOnce()
expect(cache.put).not.toHaveBeenCalled()
expect(cache.put).not.toHaveBeenCalledOnce()
})
})
61 changes: 43 additions & 18 deletions package/src/context.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {loadQuery, type QueryResponseInitial, setServerClient} from '@sanity/react-loader'
import {CacheLong, CacheNone, type CachingStrategy, createWithCache} from '@shopify/hydrogen'
import {
CacheLong,
CacheNone,
type CachingStrategy,
createWithCache,
type WithCache,
} from '@shopify/hydrogen'

import {
type ClientConfig,
Expand Down Expand Up @@ -62,10 +68,18 @@ type HydrogenResponseQueryOptions = Omit<ResponseQueryOptions, 'next' | 'cache'>
hydrogen?: 'hydrogen' extends keyof RequestInit ? RequestInit['hydrogen'] : never
}

type LoadQueryOptions = Pick<
type LoadQueryOptions<T> = Pick<
HydrogenResponseQueryOptions,
'perspective' | 'hydrogen' | 'useCdn' | 'stega' | 'headers' | 'tag'
>
> & {
hydrogen?: {
/**
* Whether to cache the result of the query or not.
* @defaultValue () => true
*/
shouldCacheResult?: (value: QueryResponseInitial<T>) => boolean
}
}

export type SanityContext = {
/**
Expand All @@ -76,7 +90,7 @@ export type SanityContext = {
loadQuery<T = any>(
query: string,
params?: QueryParams,
options?: LoadQueryOptions,
options?: LoadQueryOptions<T>,
): Promise<QueryResponseInitial<T>>

client: SanityClient
Expand Down Expand Up @@ -125,30 +139,41 @@ export function createSanityContext(options: CreateSanityContextOptions): Sanity
async loadQuery<T>(
query: string,
params: QueryParams | QueryWithoutParams,
loaderOptions?: LoadQueryOptions,
loaderOptions?: LoadQueryOptions<T>,
): Promise<QueryResponseInitial<T>> {
if (!withCache) {
return await loadQuery<T>(query, params, loaderOptions)
}

// Don't store response if preview is enabled
const cacheStrategy =
preview && preview.enabled
? CacheNone()
: loaderOptions?.hydrogen?.cache || defaultStrategy || DEFAULT_CACHE_STRATEGY

const queryHash = await hashQuery(query, params)
const shouldCacheResult = loaderOptions?.hydrogen?.shouldCacheResult ?? (() => true)

const runWithCache = async function runWithCache({
addDebugData,
}: Parameters<Parameters<WithCache['run']>[1]>[0]): Promise<QueryResponseInitial<T>> {
// eslint-disable-next-line no-process-env
if (process.env.NODE_ENV === 'development') {
// Name displayed in the subrequest profiler
const displayName = loaderOptions?.hydrogen?.debug?.displayName || 'query Sanity'

addDebugData({
displayName,
})
}

return await (withCache
? withCache(queryHash, cacheStrategy, async ({addDebugData}) => {
if (process.env.NODE_ENV === 'development') {
// Name displayed in the subrequest profiler
const displayName = loaderOptions?.hydrogen?.debug?.displayName || 'query Sanity'

addDebugData({
displayName,
})
}
return await loadQuery<T>(query, params, loaderOptions)
}

return await loadQuery<T>(query, params, loaderOptions)
})
: loadQuery<T>(query, params, loaderOptions))
return await ('run' in withCache
? withCache.run({cacheKey: queryHash, cacheStrategy, shouldCacheResult}, runWithCache)
: // @ts-expect-error for compatibility, remove in next major
withCache(queryHash, cacheStrategy, runWithCache))
},
client,
preview,
Expand Down
22 changes: 15 additions & 7 deletions package/src/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ function waitUntil() {
return Promise.resolve()
}

type WithCache = ReturnType<typeof createWithCache>
const withCache: WithCache = vi.fn().mockImplementation(createWithCache({cache, waitUntil}))
const withCache = createWithCache({cache, waitUntil, request: new Request('https://example.com')})
const runWithCache = vi.spyOn(withCache, 'run')

const client = createClient({projectId: 'my-project-id', dataset: 'my-dataset'})

Expand Down Expand Up @@ -63,15 +63,23 @@ describe('the loader', () => {
})

await loaderWithDefaultStrategy.loadQuery<boolean>(query, params)
expect(withCache).toHaveBeenCalledWith(hashedQuery, strategy, expect.any(Function))
expect(cache.put).toHaveBeenCalled()
expect(runWithCache).toHaveBeenNthCalledWith(
1,
expect.objectContaining({cacheKey: hashedQuery, cacheStrategy: strategy}),
expect.any(Function),
)
expect(cache.put).toHaveBeenCalledOnce()
})

it('queries should use the cache strategy passed in `loadQuery`', async () => {
const strategy = CacheShort()
await loader.loadQuery<boolean>(query, params, {hydrogen: {cache: strategy}})
expect(withCache).toHaveBeenCalledWith(hashedQuery, strategy, expect.any(Function))
expect(cache.put).toHaveBeenCalled()
expect(runWithCache).toHaveBeenNthCalledWith(
1,
expect.objectContaining({cacheKey: hashedQuery, cacheStrategy: strategy}),
expect.any(Function),
)
expect(cache.put).toHaveBeenCalledOnce()
})
})

Expand Down Expand Up @@ -108,6 +116,6 @@ describe('when configured for preview', () => {
it(`shouldn't cache queries`, async () => {
await previewLoader.loadQuery<boolean>(query)
expect(loadQuery).toHaveBeenCalledOnce()
expect(cache.put).not.toHaveBeenCalled()
expect(cache.put).not.toHaveBeenCalledOnce()
})
})
29 changes: 23 additions & 6 deletions package/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,18 @@ type HydrogenResponseQueryOptions = Omit<ResponseQueryOptions, 'next' | 'cache'>
hydrogen?: 'hydrogen' extends keyof RequestInit ? RequestInit['hydrogen'] : never
}

type LoadQueryOptions = Pick<
type LoadQueryOptions<T> = Pick<
HydrogenResponseQueryOptions,
'perspective' | 'hydrogen' | 'useCdn' | 'stega' | 'headers' | 'tag'
>
> & {
hydrogen?: {
/**
* Whether to cache the result of the query or not.
* @defaultValue () => true
*/
shouldCacheResult?: (value: QueryResponseInitial<T>) => boolean
}
}

export type SanityLoader = {
/**
Expand All @@ -75,7 +83,7 @@ export type SanityLoader = {
loadQuery<T = any>(
query: string,
params?: QueryParams,
options?: LoadQueryOptions,
options?: LoadQueryOptions<T>,
): Promise<QueryResponseInitial<T>>

client: SanityClient
Expand Down Expand Up @@ -123,7 +131,7 @@ export function createSanityLoader(options: CreateSanityLoaderOptions): SanityLo
async loadQuery<T>(
query: string,
params: QueryParams | QueryWithoutParams,
loaderOptions?: LoadQueryOptions,
loaderOptions?: LoadQueryOptions<T>,
): Promise<QueryResponseInitial<T>> {
// Don't store response if preview is enabled
const cacheStrategy =
Expand All @@ -133,7 +141,11 @@ export function createSanityLoader(options: CreateSanityLoaderOptions): SanityLo

const queryHash = await hashQuery(query, params)

return await withCache(queryHash, cacheStrategy, async ({addDebugData}) => {
const shouldCacheResult = loaderOptions?.hydrogen?.shouldCacheResult ?? (() => true)

const runWithCache = async function runWithCache({
addDebugData,
}: Parameters<Parameters<WithCache['run']>[1]>[0]): Promise<QueryResponseInitial<T>> {
// eslint-disable-next-line no-process-env
if (process.env.NODE_ENV === 'development') {
// Name displayed in the subrequest profiler
Expand All @@ -145,7 +157,12 @@ export function createSanityLoader(options: CreateSanityLoaderOptions): SanityLo
}

return await loadQuery<T>(query, params, loaderOptions)
})
}

return await ('run' in withCache
? withCache.run({cacheKey: queryHash, cacheStrategy, shouldCacheResult}, runWithCache)
: // @ts-expect-error for compatibility, remove in next major
withCache(queryHash, cacheStrategy, runWithCache))
},
client,
preview,
Expand Down
Loading
Loading