chore: adds postal code as key value#3272
Conversation
WalkthroughSession postal codes are now integrated into cache-busting and query execution logic. When delivery-promise is enabled, postal code values from user sessions are tracked alongside person IDs in cache keys, and query execution is deferred during session validation to ensure region-aware caching consistency. Changes
Sequence DiagramsequenceDiagram
actor User
participant Session as Session Manager
participant StoreConfig as Store Config
participant Cache as Cache Storage
participant Query as Query Hook
User->>Session: Load page with session
Session->>Session: Extract personId & postalCode
alt Delivery Promise Enabled
Session->>StoreConfig: Check deliveryPromise.enabled
StoreConfig-->>Session: true
Session->>Query: isSessionValidating = true
Query->>Query: doNotRun = true
Query-->>User: Query deferred (awaiting session)
Session->>Session: Session validation complete
Session->>Query: isSessionValidating = false
Query->>Query: doNotRun = false
Query->>Query: Build localizedVariablesWithRegion<br/>(includes _postalCode)
Query->>Cache: Check cache with postal-aware key
alt Cache Miss
Query->>Cache: Generate new cache-bust value<br/>(timestamp::personId::postalCode)
Cache-->>Query: Store updated values
Query-->>User: Execute query with region context
else Cache Hit
Cache-->>Query: Reuse cached value
Query-->>User: Return cached result
end
else Delivery Promise Disabled
Query->>Query: Use localizedVariables only
Query-->>User: Execute query normally
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/sdk/graphql/useQuery.ts (1)
49-50: Extract the full SWR key builder into one helper.
getKey()still returns onlyoperationName + variables, whileuseQuery()now appends the session and postal-code suffixes inline. Sincepackages/core/src/sdk/graphql/prefetchQuery.ts:1-30andpackages/core/src/sdk/graphql/useLazyQuery.ts:1-40already depend ongetKey(), centralizing the final cache-key shape here will make future cache-aware callers harder to drift.As per coding guidelines, "Business logic should be separated from UI concerns".♻️ Suggested refactor
export const getKey = <Variables>( operationName: string, variables: Variables ) => `${operationName}::${JSON.stringify(variables)}` + +export const getSWRKey = <Variables>( + operationName: string, + variables: Variables, + { + sessionSuffix, + keySuffix, + }: { sessionSuffix?: string; keySuffix?: string } = {} +) => { + const baseKey = getKey(operationName, variables) + const extraSuffix = keySuffix ? `::${keySuffix}` : '' + + return `${baseKey}::${sessionSuffix ?? ''}${extraSuffix}` +} ... () => { if (options?.doNotRun) return null - const baseKey = getKey(operation['__meta__']['operationName'], variables) - const sessionSuffix = getSessionCacheKeySuffix() - const extra = options?.keySuffix ? `::${options.keySuffix}` : '' - return `${baseKey}::${sessionSuffix}${extra}` + return getSWRKey(operation['__meta__']['operationName'], variables, { + sessionSuffix: getSessionCacheKeySuffix(), + keySuffix: options?.keySuffix, + }) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/sdk/graphql/useQuery.ts` around lines 49 - 50, The current key construction is split: getKey() returns only operationName+variables while useQuery() appends session/postal-code suffixes inline, causing drift; extract a single helper (e.g., buildSWRKey or getFullKey) that composes baseKey, sessionSuffix and options.keySuffix and replace inline concatenation in useQuery(), useLazyQuery() and prefetchQuery to call that helper so all callers share the exact same cache-key shape (update getKey() to delegate or be replaced by the new helper and update callers to use the new symbol).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/sdk/graphql/useQuery.ts`:
- Around line 49-50: The current key construction is split: getKey() returns
only operationName+variables while useQuery() appends session/postal-code
suffixes inline, causing drift; extract a single helper (e.g., buildSWRKey or
getFullKey) that composes baseKey, sessionSuffix and options.keySuffix and
replace inline concatenation in useQuery(), useLazyQuery() and prefetchQuery to
call that helper so all callers share the exact same cache-key shape (update
getKey() to delegate or be replaced by the new helper and update callers to use
the new symbol).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6b8ee22-55f8-4b6b-ab52-a9a04006027d
📒 Files selected for processing (2)
packages/core/src/sdk/graphql/useQuery.tspackages/core/src/sdk/product/useProductGalleryQuery.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sdk/product/usePageProductsQuery.ts`:
- Around line 111-120: The seeded page cache and initialVariables are missing
the region-aware _postalCode key shape, causing SSR-loaded page 0 to miss
because hasSameVariables compares to getKey(localizedVariablesWithRegion);
update the seeding so the same key helper is used with postalCode (e.g., call
getPageCacheKey or getKey with params.serverManyProductsVariables plus
postalCode) when populating pagesCache.current and initialVariables so hydration
and subsequent deepEquals comparisons use the identical key shape (also apply
the same change at the other occurrence around line 142).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba2b3b4a-d845-4f2c-9bb2-3b4b0ae46d65
📒 Files selected for processing (2)
packages/core/src/sdk/graphql/useQuery.tspackages/core/src/sdk/product/usePageProductsQuery.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/sdk/graphql/useQuery.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/sdk/product/usePageProductsQuery.ts (1)
111-136:isDeliveryPromiseEnabledcaptured in stale closure.
useGalleryPageis wrapped inuseCallback(fn, [])with an empty dependency array, but it referencesisDeliveryPromiseEnabledfrom the outer scope (line 132). SincestoreConfigis a static import and doesn't change at runtime, this is likely fine in practice. However, if this value were ever dynamic, the closure would be stale.For clarity and correctness, consider adding
isDeliveryPromiseEnabledto the dependency array or moving the config read inside the callback:Option: move config read inside callback
const useGalleryPage = useCallback(function useGalleryPage(page: number) { + const isDeliveryPromiseEnabled = storeConfig.deliveryPromise?.enabled ?? false const { state: { sort, term, selectedFacets }, itemsPerPage, } = useSearch()Since
storeConfigis a build-time static import, this is cosmetic — no runtime bug expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/sdk/product/usePageProductsQuery.ts` around lines 111 - 136, The useGalleryPage callback captures isDeliveryPromiseEnabled from the outer scope while being memoized with an empty dependency array, risking a stale closure; update the useCallback for useGalleryPage to include isDeliveryPromiseEnabled in its dependency array, and also add isDeliveryPromiseEnabled to the dependency array of the useMemo that computes localizedVariablesWithRegion (or alternatively move the isDeliveryPromiseEnabled read inside the useCallback and inside the useMemo) so both useGalleryPage and localizedVariablesWithRegion react to changes in isDeliveryPromiseEnabled.packages/core/test/utils/cookieCacheBusting.test.ts (1)
148-218: Consider adding test fordeliveryPromisedisabled scenario.The tests mock
deliveryPromise.enabled: trueglobally. It would strengthen coverage to verify that whendeliveryPromiseis disabled,postalCodeis ignored and doesn't affect the cache key — matching the behavior ingetPostalCode()that returnsnullwhen the feature is off.Example test case
describe('when deliveryPromise is disabled', () => { beforeEach(() => { jest.resetModules() jest.doMock('discovery.config', () => ({ deliveryPromise: { enabled: false }, })) }) it('should ignore postalCode and return null for anonymous users', async () => { const { getClientCacheBustingValue } = await import( '../../src/utils/cookieCacheBusting' ) // setSession with postalCode but no personId // expect null because deliveryPromise is disabled }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/utils/cookieCacheBusting.test.ts` around lines 148 - 218, Add a test case that ensures postalCode is ignored when the deliveryPromise feature is disabled: reset modules with jest.resetModules(), mock the discovery.config via jest.doMock('discovery.config', () => ({ deliveryPromise: { enabled: false } })), then import getClientCacheBustingValue (after the mock), call setSession({ postalCode: '24230220' }) for an anonymous user, mock Date.now to a fixed value, call getClientCacheBustingValue() and assert the returned cache-busting string does not include the postalCode (i.e., behaves as if getPostalCode() returned null); reference getClientCacheBustingValue and getPostalCode to locate where the behavior should match the disabled feature flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/sdk/product/usePageProductsQuery.ts`:
- Around line 111-136: The useGalleryPage callback captures
isDeliveryPromiseEnabled from the outer scope while being memoized with an empty
dependency array, risking a stale closure; update the useCallback for
useGalleryPage to include isDeliveryPromiseEnabled in its dependency array, and
also add isDeliveryPromiseEnabled to the dependency array of the useMemo that
computes localizedVariablesWithRegion (or alternatively move the
isDeliveryPromiseEnabled read inside the useCallback and inside the useMemo) so
both useGalleryPage and localizedVariablesWithRegion react to changes in
isDeliveryPromiseEnabled.
In `@packages/core/test/utils/cookieCacheBusting.test.ts`:
- Around line 148-218: Add a test case that ensures postalCode is ignored when
the deliveryPromise feature is disabled: reset modules with jest.resetModules(),
mock the discovery.config via jest.doMock('discovery.config', () => ({
deliveryPromise: { enabled: false } })), then import getClientCacheBustingValue
(after the mock), call setSession({ postalCode: '24230220' }) for an anonymous
user, mock Date.now to a fixed value, call getClientCacheBustingValue() and
assert the returned cache-busting string does not include the postalCode (i.e.,
behaves as if getPostalCode() returned null); reference
getClientCacheBustingValue and getPostalCode to locate where the behavior should
match the disabled feature flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e16e47cb-109f-4dcf-a46c-560b5500c36d
📒 Files selected for processing (5)
packages/core/src/sdk/graphql/request.tspackages/core/src/sdk/product/usePageProductsQuery.tspackages/core/src/sdk/product/useProductGalleryQuery.tspackages/core/src/utils/cookieCacheBusting.tspackages/core/test/utils/cookieCacheBusting.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/sdk/graphql/request.ts
## What's the purpose of this pull request? On the PLP, when a user changes their postal code to one where certain delivery methods (e.g. "Shipping", "Dynamic Estimate") are unavailable, the delivery facets (Delivery Methods, Delivery Options) are not updated — the stale filters from the previous postal code remain visible. ## How it works? - cookieCacheBusting.ts — extend getClientCacheBustingValue() to track postalCode alongside person.id. The resulting v query param (e.g. v=timestamp::userId::24230220) is already appended to every GET request URL, so this single change busts both the CDN cache and the SWR client-side cache when the postal code changes. - useProductGalleryQuery.ts / usePageProductsQuery.ts — add doNotRun: isDeliveryPromiseEnabled && isSessionValidating to gate the gallery and product queries while validateSession is in flight. - usePageProductsQuery.ts — include _postalCode in the pagesCache comparison key (localizedVariablesWithRegion). This is needed so the hasSameVariables guard — which decides whether SWR is allowed to run at all — correctly detects a postal code change and sets shouldFetch = true. Without it, the local cache guard would permanently block the query for an unchanged set of search params. The SSR pagesCache seed is updated to match this shape so the first hydration render doesn't trigger an unnecessary re-fetch. ## How to test it? Run locally or check this [preview](https://vendemo-cm9sir9v900u7z6llkl62l70j-mdd18ev50.b.vtex.app) In discovery.config file: - use `vendemo` account to test ``` api: { storeId: 'vendemo', ..} ``` - enable deliveryPromise: ``` deliveryPromise: { enabled: true, mandatory: false, }, ``` You should be the filters being updated after changing the postal code. https://github.com/user-attachments/assets/bf0814e4-d863-421e-9d15-f0f8de6d87be ### Starters Deploy Preview <!--- Add a link to a deploy preview from `starter.store` with this branch being used. ---> <!--- Tip: You can get an installable version of this branch from the CodeSandbox generated when this PR is created. ---> ## References https://vtex.slack.com/archives/C0164SHMAV9/p1776083445684739 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Product queries now incorporate postal code and region information for more location-specific results and optimized data caching. * Enhanced session handling and request optimization for delivery-related features. * Cache management now simultaneously monitors authentication state and location changes for improved accuracy. * **Tests** * Extended test coverage for location-aware caching across authenticated and anonymous user scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What's the purpose of this pull request? On the PLP, when a user changes their postal code to one where certain delivery methods (e.g. "Shipping", "Dynamic Estimate") are unavailable, the delivery facets (Delivery Methods, Delivery Options) are not updated — the stale filters from the previous postal code remain visible. ## How it works? - cookieCacheBusting.ts — extend getClientCacheBustingValue() to track postalCode alongside person.id. The resulting v query param (e.g. v=timestamp::userId::24230220) is already appended to every GET request URL, so this single change busts both the CDN cache and the SWR client-side cache when the postal code changes. - useProductGalleryQuery.ts / usePageProductsQuery.ts — add doNotRun: isDeliveryPromiseEnabled && isSessionValidating to gate the gallery and product queries while validateSession is in flight. - usePageProductsQuery.ts — include _postalCode in the pagesCache comparison key (localizedVariablesWithRegion). This is needed so the hasSameVariables guard — which decides whether SWR is allowed to run at all — correctly detects a postal code change and sets shouldFetch = true. Without it, the local cache guard would permanently block the query for an unchanged set of search params. The SSR pagesCache seed is updated to match this shape so the first hydration render doesn't trigger an unnecessary re-fetch. ## How to test it? Run locally or check this [preview](https://vendemo-cm9sir9v900u7z6llkl62l70j-mdd18ev50.b.vtex.app) In discovery.config file: - use `vendemo` account to test ``` api: { storeId: 'vendemo', ..} ``` - enable deliveryPromise: ``` deliveryPromise: { enabled: true, mandatory: false, }, ``` You should be the filters being updated after changing the postal code. https://github.com/user-attachments/assets/bf0814e4-d863-421e-9d15-f0f8de6d87be ### Starters Deploy Preview <!--- Add a link to a deploy preview from `starter.store` with this branch being used. ---> <!--- Tip: You can get an installable version of this branch from the CodeSandbox generated when this PR is created. ---> ## References https://vtex.slack.com/archives/C0164SHMAV9/p1776083445684739 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Product queries now incorporate postal code and region information for more location-specific results and optimized data caching. * Enhanced session handling and request optimization for delivery-related features. * Cache management now simultaneously monitors authentication state and location changes for improved accuracy. * **Tests** * Extended test coverage for location-aware caching across authenticated and anonymous user scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What's the purpose of this pull request?
On the PLP, when a user changes their postal code to one where certain delivery methods (e.g. "Shipping", "Dynamic Estimate") are unavailable, the delivery facets (Delivery Methods, Delivery Options) are not updated — the stale filters from the previous postal code remain visible.
How it works?
cookieCacheBusting.ts — extend getClientCacheBustingValue() to track postalCode alongside person.id. The resulting v query param (e.g. v=timestamp::userId::24230220) is already appended to every GET request URL, so this single change busts both the CDN cache and the SWR client-side cache when the postal code changes.
useProductGalleryQuery.ts / usePageProductsQuery.ts — add doNotRun: isDeliveryPromiseEnabled && isSessionValidating to gate the gallery and product queries while validateSession is in flight.
usePageProductsQuery.ts — include _postalCode in the pagesCache comparison key (localizedVariablesWithRegion). This is needed so the hasSameVariables guard — which decides whether SWR is allowed to run at all — correctly detects a postal code change and sets shouldFetch = true. Without it, the local cache guard would permanently block the query for an unchanged set of search params. The SSR pagesCache seed is updated to match this shape so the first hydration render doesn't trigger an unnecessary re-fetch.
How to test it?
Run locally or check this preview
In discovery.config file:
vendemoaccount to testYou should be the filters being updated after changing the postal code.
facets-products-updating.mov
Starters Deploy Preview
References
https://vtex.slack.com/archives/C0164SHMAV9/p1776083445684739
Summary by CodeRabbit
Release Notes
New Features
Tests