feat(core): add RecommendationShelf section with personalization tracking#3403
feat(core): add RecommendationShelf section with personalization tracking#3403thiagopereira-vtex wants to merge 30 commits into
Conversation
…king Adds a CMS-configurable RecommendationShelf section that renders VTEX personalization recommendations using the shared ProductCard, plus a global useStartSession hook (mounted in Layout) that starts the anonymous personalization session once per browser session and reports PDP product views.
thiagopereira-vtex
requested review from
emersonlaurentino and
hellofanny
and removed request for
a team
thiagopereira-vtex
added
the
contributing
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds recommendation GraphQL contracts and VTEX wiring, shelf support hooks and rendering, CMS registration, and client-side session startup with product metadata updates. ChangesRecommendation shelf and session flow
Sequence Diagram(s)Recommendation shelf flow sequenceDiagram
participant RecommendationShelf
participant useRecommendationUserId
participant usePDP
participant useRecommendations
participant ProductShelfSkeleton
participant Carousel
participant ProductCard
RecommendationShelf->>useRecommendationUserId: resolve userId
RecommendationShelf->>usePDP: read productGroupID
RecommendationShelf->>useRecommendations: fetch campaignVrn and products
useRecommendations-->>RecommendationShelf: data / error / isLoading
alt loading
RecommendationShelf->>ProductShelfSkeleton: render loading state
else loaded
RecommendationShelf->>Carousel: render shelf
loop products
RecommendationShelf->>ProductCard: render mapped card
end
end
Recommendation session startup sequenceDiagram
participant Layout
participant useStartRecommendationSession
participant getCookie
participant retry
participant runStartRecommendationSession
participant startRecommendationSession
Layout->>useStartRecommendationSession: call after usePageViewEvent(props)
useStartRecommendationSession->>getCookie: read vtex-rec-user-start-session
alt cookie missing
useStartRecommendationSession->>retry: retry session start
retry->>runStartRecommendationSession: execute mutation
runStartRecommendationSession->>startRecommendationSession: start session
startRecommendationSession-->>runStartRecommendationSession: Boolean!
runStartRecommendationSession-->>retry: defined result
else cookie present
useStartRecommendationSession-->>Layout: skip
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
packages/core/src/components/RecommendationShelf/vrn.ts (1)
22-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid unsafe casting for
campaignVrnTypeduring parsing.
campaignType as RecommendationVrnTypesuppresses type safety at the boundary. Validate and narrow before returning typed data so invalid VRNs fail in a typed path, not through a late fallback.♻️ Suggested refactor
+const VRN_TYPES = new Set<RecommendationVrnType>([ + 'rec-cross-v2', + 'rec-similar-v2', + 'rec-persona-v2', + 'rec-last-v2', + 'rec-top-items-v2', + 'rec-search-v2', + 'rec-next-v2', + 'rec-visual-v2', +]) + function parseCampaignVrn(campaignVrn: string) { const [_, __, accountName, campaignType, campaignId] = campaignVrn.split(':') + if (!campaignType || !VRN_TYPES.has(campaignType as RecommendationVrnType)) { + throw new Error(`Unknown campaign type: ${campaignType}`) + } return { accountName, campaignId, - campaignVrnType: campaignType as RecommendationVrnType, + campaignVrnType: campaignType, } }As per path instructions,
packages/**/*.{ts,tsx}should "Ensure type safety and avoid type assertions when possible."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/vrn.ts` around lines 22 - 29, The `parseCampaignVrn` function uses an unsafe type assertion `campaignType as RecommendationVrnType` which bypasses type safety at the boundary. Instead of casting, validate that the extracted `campaignType` value is actually a valid `RecommendationVrnType` before returning it. Implement validation logic such as checking against the allowed enum or type values, and throw an error or return a failure result if the value is invalid, ensuring type safety is maintained throughout the parsing path without relying on assertions.Source: Path instructions
packages/core/src/components/RecommendationShelf/useRecommendations.ts (2)
165-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the type assertion fallback for query variables.
Line 165 uses
({} as RecommendationInput), which weakens type safety. Prefer a typed default object or updateuseQueryto accept nullable variables withdoNotRun.As per path instructions,
packages/**/*.{ts,tsx}: “Ensure type safety and avoid type assertions when possible”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/useRecommendations.ts` at line 165, In the useRecommendations hook, the useQuery call on line 165 uses a type assertion ({} as RecommendationInput) when args is undefined, which weakens type safety. Replace this type assertion with either a properly typed default object that matches the RecommendationInput structure, or refactor the useQuery invocation to use the doNotRun option to handle cases where variables are nullable instead of relying on the assertion fallback.Source: Path instructions
15-144: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTrim the query to only fields consumed by RecommendationShelf/ProductCard mapping.
This selection set is much larger than what Line 170+ downstream mapping/rendering needs, increasing payload and client work on PDP. Keep only fields used by
mapRecommendationToProductCardand card rendering.As per path instructions,
packages/**/src/components/**: “Check rendering performance (avoid heavy operations on render...)”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/useRecommendations.ts` around lines 15 - 144, The GraphQL query in the products selection set is requesting many fields that are not consumed by the downstream mapRecommendationToProductCard function and card rendering logic. Review the mapRecommendationToProductCard function and the ProductCard component rendering to identify which fields from the products query are actually used, then remove all unused fields from the products selection set. Keep only the fields that are directly referenced in the mapping function and rendering logic to reduce payload size and improve rendering performance.Source: Path instructions
packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlias
@faststore/uiimports withUIprefix in core components.This file imports
ProductShelfandCarouseldirectly from@faststore/ui. In this path, please useUIProductShelf,UICarousel, etc., to match the local composition convention.As per coding guidelines,
packages/core/src/components/**/*.{ts,tsx}: “Import from@faststore/uiand alias withUIprefix to avoid local name collisions when composing library components”.Also applies to: 136-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx` at line 4, Update the import statement at line 4 in RecommendationShelf.tsx to alias ProductShelf and Carousel with the UI prefix by using the `as` keyword (e.g., `import { ProductShelf as UIProductShelf, Carousel as UICarousel }`). Then update all usages of ProductShelf and Carousel throughout the component to use their new aliased names UIProductShelf and UICarousel. Also apply the same aliasing pattern to any other imports from `@faststore/ui` in the file, particularly at lines 136-141, to follow the core components naming convention and avoid local name collisions.Source: Coding guidelines
packages/core/src/components/RecommendationShelf/index.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRe-export component props/types from the barrel.
This barrel exports only the component symbol. Please also re-export
RecommendationShelfProps(and any future variant types) so consumers can type against the barrel path.As per coding guidelines,
packages/{components,core}/src/components/**/*.{ts,tsx}: “AllPropsand variant types MUST be re-exported from the barrelindex.ts”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/index.ts` at line 1, The barrel index.ts file in the RecommendationShelf component directory is only re-exporting the component itself, but not the associated prop types. Update the index.ts file to also explicitly re-export RecommendationShelfProps and any variant types that exist in the RecommendationShelf module. This ensures consumers can import and use the component props by importing directly from the barrel path, following the coding guidelines that require all Props and variant types to be re-exported from barrel index.ts files in the components directory.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/core/cms/faststore/components/cms_component__RecommendationShelf.jsonc`:
- Around line 13-16: The `campaignVrn` property definition in the schema has an
example format in its description that does not match the VRN format accepted by
the `RecommendationShelf/vrn.ts` runtime parser, allowing users to configure
invalid values that fail at runtime. Update the example in the description field
to match the correct VRN format expected by the parser, and add a `pattern`
constraint to the `campaignVrn` property schema to validate the VRN format
during configuration time, preventing invalid values from being accepted in the
CMS.
In `@packages/core/src/components/RecommendationShelf/queries/request.ts`:
- Around line 51-62: The code currently defaults to using GET requests for
GraphQL queries and places variables (including userId) in the URL query string,
which creates a security risk for information leakage. Change the method
selection logic to use POST as the default instead of GET, and either remove the
GET conditional path entirely or create an explicit whitelist of safe operations
that do not contain personalization data. Update the method variable assignment
in the conditional expression to prioritize POST, and ensure that the
URLSearchParams construction does not include variables when using GET requests.
- Around line 85-106: The baseRequest function can return null when the response
content type is unrecognized (neither JSON nor plain text), causing callers that
destructure the response to fail. Additionally, there is a duplicate if
(response.ok) block that is unreachable. Remove the second if (response.ok)
block entirely, and in the first block, ensure that when responseBody is null,
provide a fallback by calling response.json() to attempt parsing the response
body as JSON, so that a non-null value is always returned on a successful
response.
In `@packages/core/src/components/RecommendationShelf/queries/useLazyQuery.ts`:
- Around line 12-29: The useLazyQuery hook has a cache key mismatch where the
initial cache key is generated using the initial variables parameter (in the
getKey call), but the execute function accepts queryVariables which may differ.
When execute calls response.mutate with different queryVariables than what
initialized the hook, the cache update happens under the wrong key. Fix this by
either dynamically generating the cache key within the execute function using
the queryVariables parameter instead of the initial variables, or by
restructuring the code to ensure the SWR cache key always matches the variables
actually being used for the request. The key should be recalculated based on
whatever variables are passed to execute so that response.mutate updates the
correct cache entry.
In `@packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx`:
- Around line 103-110: The console.error call in the error handling block logs
the entire recommendationArgs object, which contains sensitive user identifiers
like userId. Instead of logging the full recommendationArgs parameter, replace
it with a sanitized version that only includes non-sensitive metadata such as
campaign type or correlation id presence. Extract and log only the safe,
non-identifying fields from recommendationArgs to protect user privacy while
still maintaining useful debugging information.
- Around line 84-101: The async cookie retrieval logic with getWithRetry is
currently running during render, which causes it to execute on every render and
be duplicated in React Strict Mode. Move the entire conditional block that calls
getWithRetry and setUserId into a useEffect hook with userId as a dependency.
This ensures the async operation only runs once when userId becomes available,
preventing duplicate retries and unnecessary state updates.
In `@packages/core/src/components/RecommendationShelf/useRecommendations.ts`:
- Around line 5-153: The GraphQL query in the gql template at the root level is
attempting to query a field called "recommendations" that does not exist on the
Query type according to the schema, causing schema validation to fail. Identify
the correct query field name by consulting the GraphQL schema documentation or
backend schema definition, then replace the "recommendations" field name in the
query with the actual field name that exists in the schema. Ensure all the
nested fields and variables being queried match what the actual schema field
definition expects.
In `@packages/core/src/sdk/analytics/hooks/useStartSession.ts`:
- Around line 5-9: Move the utility functions checkIsMobile and
getUserIdFromCookie from the RecommendationShelf component module to an
appropriate SDK-level utility module (create a new one in src/sdk if needed).
Update the import statements in useStartSession.ts to import these functions
from the new SDK utility location instead of from
src/components/RecommendationShelf/utils. This ensures the analytics SDK logic
remains decoupled from UI component modules.
- Around line 131-137: The issue is that lastProductViewSentRef.current is being
set before the runSendProductViewEvent mutation completes. If the network call
fails or times out, the ref retains the productId value and prevents any future
retry attempts for that product view. Move the assignment of
lastProductViewSentRef.current = productId to execute only after the
runSendProductViewEvent call succeeds by either awaiting the promise result and
updating the ref in a success callback, or by removing the void annotation and
properly handling the promise chain to ensure the ref is only updated on
successful completion.
- Around line 60-63: Replace the `any` type for the `props` parameter in the
`useStartSession` function with a properly typed interface that captures the
nested structure being accessed. Define a minimal type that includes the path
`data?.product?.isVariantOf?.productGroupID` with appropriate property types
(productGroupID should be a string), then update the function signature to use
this new type instead of `any` to restore compile-time type safety.
- Around line 16-30: In the useStartSession hook, refactor the imports and
logic: first, change the import of useLazyQuery to come from
src/sdk/graphql/useLazyQuery instead of the component folder path, second,
extract checkIsMobile and getUserIdFromCookie utility functions to
src/sdk/analytics/utils.ts and import them from there, third, replace the
props?: any parameter with a properly typed interface similar to the pattern
used in usePageViewEvent.ts to ensure type safety, and fourth, fix the
deduplication logic by moving the assignment of lastProductViewSentRef.current =
productId to execute only after the sendProductViewEventMutation succeeds
(inside the success callback of runSendProductViewEvent) rather than before the
mutation call, ensuring the ref is only updated when the mutation actually
completes.
---
Nitpick comments:
In `@packages/core/src/components/RecommendationShelf/index.ts`:
- Line 1: The barrel index.ts file in the RecommendationShelf component
directory is only re-exporting the component itself, but not the associated prop
types. Update the index.ts file to also explicitly re-export
RecommendationShelfProps and any variant types that exist in the
RecommendationShelf module. This ensures consumers can import and use the
component props by importing directly from the barrel path, following the coding
guidelines that require all Props and variant types to be re-exported from
barrel index.ts files in the components directory.
In `@packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx`:
- Line 4: Update the import statement at line 4 in RecommendationShelf.tsx to
alias ProductShelf and Carousel with the UI prefix by using the `as` keyword
(e.g., `import { ProductShelf as UIProductShelf, Carousel as UICarousel }`).
Then update all usages of ProductShelf and Carousel throughout the component to
use their new aliased names UIProductShelf and UICarousel. Also apply the same
aliasing pattern to any other imports from `@faststore/ui` in the file,
particularly at lines 136-141, to follow the core components naming convention
and avoid local name collisions.
In `@packages/core/src/components/RecommendationShelf/useRecommendations.ts`:
- Line 165: In the useRecommendations hook, the useQuery call on line 165 uses a
type assertion ({} as RecommendationInput) when args is undefined, which weakens
type safety. Replace this type assertion with either a properly typed default
object that matches the RecommendationInput structure, or refactor the useQuery
invocation to use the doNotRun option to handle cases where variables are
nullable instead of relying on the assertion fallback.
- Around line 15-144: The GraphQL query in the products selection set is
requesting many fields that are not consumed by the downstream
mapRecommendationToProductCard function and card rendering logic. Review the
mapRecommendationToProductCard function and the ProductCard component rendering
to identify which fields from the products query are actually used, then remove
all unused fields from the products selection set. Keep only the fields that are
directly referenced in the mapping function and rendering logic to reduce
payload size and improve rendering performance.
In `@packages/core/src/components/RecommendationShelf/vrn.ts`:
- Around line 22-29: The `parseCampaignVrn` function uses an unsafe type
assertion `campaignType as RecommendationVrnType` which bypasses type safety at
the boundary. Instead of casting, validate that the extracted `campaignType`
value is actually a valid `RecommendationVrnType` before returning it. Implement
validation logic such as checking against the allowed enum or type values, and
throw an error or return a failure result if the value is invalid, ensuring type
safety is maintained throughout the parsing path without relying on assertions.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: 01045e87-6903-4f0a-8704-6c901d1ab199
⛔ Files ignored due to path filters (2)
packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (15)
packages/core/cms/faststore/components/cms_component__RecommendationShelf.jsoncpackages/core/src/Layout.tsxpackages/core/src/components/RecommendationShelf/RecommendationShelf.module.scsspackages/core/src/components/RecommendationShelf/RecommendationShelf.tsxpackages/core/src/components/RecommendationShelf/RecommendationShelf.types.tspackages/core/src/components/RecommendationShelf/index.tspackages/core/src/components/RecommendationShelf/mapRecommendationToProductCard.tspackages/core/src/components/RecommendationShelf/queries/request.tspackages/core/src/components/RecommendationShelf/queries/useLazyQuery.tspackages/core/src/components/RecommendationShelf/queries/useQuery.tspackages/core/src/components/RecommendationShelf/useRecommendations.tspackages/core/src/components/RecommendationShelf/utils.tspackages/core/src/components/RecommendationShelf/vrn.tspackages/core/src/components/cms/global/Components.tspackages/core/src/sdk/analytics/hooks/useStartSession.ts
Codegen regenerates @generated/schema.graphql from the API plus the src/graphql customization folders, so the operations used by the RecommendationShelf (recommendations, startSession, sendProductViewEvent) must be defined as thirdParty typeDefs to be valid in CI. Adds the schema extension and the recommendation types (referencing existing Advertisement and SkuSpecification), fixing the @faststore/core#generate build failure.
|
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. |
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
Adds recommendations, startSession and sendProductViewEvent to the expected query/mutation lists in the server schema test, reflecting the new thirdParty typeDefs.
Defines the recommendations query, the startSession and sendProductViewEvent mutations and their types (RecommendationResponse, RecommendationProduct, ProductViewSource, etc.) in the VTEX platform typeDefs. These are operations shipped by @faststore/core, so they must live in @faststore/api to be part of the schema generated for any consuming store (store-level src/graphql customizations are not visible to core features). Updates the schema assertion tests accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/platforms/vtex/typeDefs/query.graphql`:
- Around line 473-481: The recommendations field in the GraphQL query schema is
marked with `@cacheControl`(scope: "public", ...) but returns personalized data
based on userId and campaign context, which can expose one shopper's
recommendations to others through shared caches. Change the scope from "public"
to "private" in the `@cacheControl` directive on the recommendations field to
ensure personalized responses are not cached in shared caches that could be
accessed by different users.
🪄 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: 3274e94b-f00c-4810-be61-0a8b7bbcb4fe
⛔ Files ignored due to path filters (2)
packages/api/src/__generated__/schema.tsis excluded by!**/__generated__/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (5)
packages/api/src/platforms/vtex/typeDefs/mutation.graphqlpackages/api/src/platforms/vtex/typeDefs/query.graphqlpackages/api/src/platforms/vtex/typeDefs/recommendation.graphqlpackages/api/test/integration/schema.test.tspackages/core/test/server/index.test.ts
💤 Files with no reviewable changes (1)
- packages/api/src/platforms/vtex/typeDefs/recommendation.graphql
Addresses SonarQube findings on the new recommendation code: replaces negated conditions and nested ternaries with early returns, uses optional chaining, removes a dead duplicated branch in the request helper, drops a redundant always-falsy check and uses globalThis.window for the SSR guard. No behavior change.
Adds unit and component tests for the recommendation feature (vrn parsing, utils, product mapping, request/query helpers, useRecommendations, useStartSession and the RecommendationShelf component), bringing coverage on the new code above the 80% quality gate.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/test/sdk/analytics/useStartSession.test.ts (1)
100-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep this test scoped to the “no product” behavior.
Line 106 asserts
runStartSessionwas not called, but that outcome is driven by the session cookie setup, not by missing product data. This adds unrelated coupling and can make the test fail for the wrong reason.Suggested cleanup
it('does not send a product view when there is no product', async () => { getCookie.mockReturnValue('already-started') renderHook(() => useStartSession({ data: {} })) - await waitFor(() => { - expect(runStartSession).not.toHaveBeenCalled() - }) expect(runSendProductViewEvent).not.toHaveBeenCalled() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/test/sdk/analytics/useStartSession.test.ts` around lines 100 - 107, The test "does not send a product view when there is no product" is tightly coupled to the session cookie setup (getCookie.mockReturnValue('already-started')) which is unrelated to testing the "no product" behavior. Remove the getCookie mock setup so that the test specifically validates that runStartSession is not called due to the missing product data in the data object, not due to session state. This keeps the test focused on its intended purpose and prevents it from failing for the wrong reason.packages/core/test/components/RecommendationShelf/mapRecommendationToProductCard.test.ts (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
as anyfixtures in TS tests to preserve type safetyThese broad assertions weaken compile-time guarantees in a critical mapping test. Prefer a typed fixture helper (e.g.,
createRecommendationProduct(overrides: Partial<RecommendationProduct>)) so invalid fixture shapes are caught during test authoring.
As per path instructions: “TypeScript files: Ensure type safety and avoid type assertions when possible.”Also applies to: 69-69, 78-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/test/components/RecommendationShelf/mapRecommendationToProductCard.test.ts` at line 30, The test file uses `as any` type assertions when creating test fixtures (appearing at lines 30, 69, and 78), which weakens type safety and allows invalid fixture shapes to slip through undetected. Create a typed fixture helper function named `createRecommendationProduct` that accepts an optional `Partial<RecommendationProduct>` parameter for overrides and returns a fully typed `RecommendationProduct` object with sensible defaults. Then replace all instances where `} as any` appears with calls to this helper function, passing any necessary overrides as arguments, thereby eliminating the type assertions and ensuring compile-time validation of fixture shapes.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/test/components/RecommendationShelf/request.test.ts`:
- Around line 96-104: The test assertion for the explicit method option is
checking for lowercase 'put', but the request implementation normalizes the
method to uppercase before calling fetch. Update the expect statement in the
"honors an explicit method from fetchOptions" test to check for 'PUT' instead of
'put' to match the actual behavior of the baseRequest function which uppercases
the fetchOptions.method value.
---
Nitpick comments:
In
`@packages/core/test/components/RecommendationShelf/mapRecommendationToProductCard.test.ts`:
- Line 30: The test file uses `as any` type assertions when creating test
fixtures (appearing at lines 30, 69, and 78), which weakens type safety and
allows invalid fixture shapes to slip through undetected. Create a typed fixture
helper function named `createRecommendationProduct` that accepts an optional
`Partial<RecommendationProduct>` parameter for overrides and returns a fully
typed `RecommendationProduct` object with sensible defaults. Then replace all
instances where `} as any` appears with calls to this helper function, passing
any necessary overrides as arguments, thereby eliminating the type assertions
and ensuring compile-time validation of fixture shapes.
In `@packages/core/test/sdk/analytics/useStartSession.test.ts`:
- Around line 100-107: The test "does not send a product view when there is no
product" is tightly coupled to the session cookie setup
(getCookie.mockReturnValue('already-started')) which is unrelated to testing the
"no product" behavior. Remove the getCookie mock setup so that the test
specifically validates that runStartSession is not called due to the missing
product data in the data object, not due to session state. This keeps the test
focused on its intended purpose and prevents it from failing for the wrong
reason.
🪄 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: 2cdb346c-3468-4629-ac8a-4d20bb3421a7
📒 Files selected for processing (12)
packages/core/src/components/RecommendationShelf/RecommendationShelf.tsxpackages/core/src/components/RecommendationShelf/queries/request.tspackages/core/src/components/RecommendationShelf/queries/useQuery.tspackages/core/src/components/RecommendationShelf/utils.tspackages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsxpackages/core/test/components/RecommendationShelf/mapRecommendationToProductCard.test.tspackages/core/test/components/RecommendationShelf/queries.test.tspackages/core/test/components/RecommendationShelf/request.test.tspackages/core/test/components/RecommendationShelf/useRecommendations.test.tspackages/core/test/components/RecommendationShelf/utils.test.tspackages/core/test/components/RecommendationShelf/vrn.test.tspackages/core/test/sdk/analytics/useStartSession.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/components/RecommendationShelf/utils.ts
- packages/core/src/components/RecommendationShelf/queries/useQuery.ts
- packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx
- packages/core/src/components/RecommendationShelf/queries/request.ts
Resolves the remaining SonarQube finding: globalThis.window is always a valid reference, so the typeof guard is unnecessary.
…dationShelf.jsonc Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
The recommendations query is personalized (userId + campaign context), so a public/shared cache scope could leak one shopper's response to others via a CDN. Switch it to scope "private".
- Fix malformed JSON in the RecommendationShelf CMS schema (stray brace). - Move the userId cookie retry into a useEffect so it no longer kicks off async work and setState during render. - Stop logging recommendation args (which carry the userId) on fetch errors; log only non-identifying context. - In useStartSession, only mark a product view as sent after the mutation succeeds, so a failed request isn't permanently suppressed. - Extend tests to cover the new failure paths.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Decouples the useStartSession SDK hook from a UI component module: checkIsMobile and getUserIdFromCookie now live in src/sdk/analytics/utils. The component-level utils keeps only getWithRetry. Tests updated/split accordingly.
Point useRecommendations and useStartSession at src/sdk/graphql/* (persisted-query GET allowlist, cache busting, session-aware cache keys) and drop the duplicated RecommendationShelf/queries folder. Also type the useStartSession props instead of `any`.
…urable Expose carouselConfiguration (variant, infiniteMode, controls and per-viewport itemsPerPageDesktop/itemsPerPageMobile) and productCardConfiguration (showDiscountBadge, bordered) through the CMS, and add a code-level mapProductToProductCard override (defaults to mapRecommendationToProductCard, not exposed via CMS). Regenerate cms/faststore/schema.json accordingly.
🛡️ SDD Check — action requiredI couldn't detect an SDD in this PR. Please check one option below (requires write access to the repo):
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/test/sdk/analytics/utils.test.ts`:
- Around line 1-3: The test file is forcing jsdom in a `.test.ts` spec, but
`utils.test.ts` should run in Node unless it is explicitly a browser test.
Remove the `@vitest-environment jsdom` directive from the top of this file if
the tests only need stubbed `window`, or rename the spec to
`utils.browser.test.ts` if it truly requires browser behavior; use the
`@vitest-environment` header and the `utils.test.ts`/`browser.test` naming
convention as the key places to adjust.
🪄 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: 178c070a-ffa2-4b68-a166-65a949d0d5b3
⛔ Files ignored due to path filters (1)
packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (13)
packages/core/cms/faststore/components/cms_component__RecommendationShelf.jsoncpackages/core/cms/faststore/schema.jsonpackages/core/src/components/RecommendationShelf/RecommendationShelf.tsxpackages/core/src/components/RecommendationShelf/RecommendationShelf.types.tspackages/core/src/components/RecommendationShelf/useRecommendations.tspackages/core/src/components/RecommendationShelf/utils.tspackages/core/src/sdk/analytics/hooks/useStartSession.tspackages/core/src/sdk/analytics/utils.tspackages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsxpackages/core/test/components/RecommendationShelf/useRecommendations.test.tspackages/core/test/components/RecommendationShelf/utils.test.tspackages/core/test/sdk/analytics/useStartSession.test.tspackages/core/test/sdk/analytics/utils.test.ts
💤 Files with no reviewable changes (1)
- packages/core/src/components/RecommendationShelf/utils.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/sdk/analytics/utils.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/core/src/components/RecommendationShelf/RecommendationShelf.types.ts
- packages/core/test/components/RecommendationShelf/useRecommendations.test.ts
- packages/core/src/components/RecommendationShelf/useRecommendations.ts
- packages/core/src/sdk/analytics/hooks/useStartSession.ts
- packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx
- packages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsx
- packages/core/test/sdk/analytics/useStartSession.test.ts
Eliminate the SendProductViewEvent mutation and its associated logic from the useStartSession hook. The product view event is now captured through meta tags on the PDP, streamlining the session management process. Update tests to reflect these changes and ensure proper functionality without the removed mutation.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/test/sdk/analytics/useStartSession.test.ts (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a retry-path test for the new loop.
useStartSession()now retries untilrunStartSession()returns a defined result, but this setup only exercises first-attempt success. A fake-timers test forundefined -> successwould lock the backoff behavior and catch regressions in the new control flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/test/sdk/analytics/useStartSession.test.ts` at line 24, Add a retry-path test for useStartSession() that covers the new loop when runStartSession() first returns undefined and then succeeds, since the current mockReturnValue only tests immediate success. Use fake timers in the existing useStartSession.test.ts suite to advance the backoff between attempts, and assert that the hook keeps retrying until a defined result is returned from runStartSession() via useLazyQueryMock.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/pages/`[slug]/p.tsx:
- Around line 226-229: The structured data in the [slug]/p.tsx page is
hard-coding the product:condition tag to new, which can misreport refurbished or
used items. Update the SEO/structured-data generation path in the page’s
metadata logic to read the real condition from the product data already
available via offers.offers[].itemCondition, and only emit the product:condition
tag when a valid condition exists. Use the existing page/data builder symbols in
this file to locate and adjust the tag construction.
- Around line 202-205: The `product:id` meta tag in `Page` is using `product.id`
instead of the previous PDP tracking identifier, which can break recommendation
attribution. Update the `content` value in the meta-tag/SEO tag block to use
`product.isVariantOf?.productGroupID` unless the contract was intentionally
changed, and keep the existing `Page` structured-data/meta-tag flow aligned with
the prior PDP product-view path.
---
Nitpick comments:
In `@packages/core/test/sdk/analytics/useStartSession.test.ts`:
- Line 24: Add a retry-path test for useStartSession() that covers the new loop
when runStartSession() first returns undefined and then succeeds, since the
current mockReturnValue only tests immediate success. Use fake timers in the
existing useStartSession.test.ts suite to advance the backoff between attempts,
and assert that the hook keeps retrying until a defined result is returned from
runStartSession() via useLazyQueryMock.
🪄 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: 6cb5a061-6559-4bae-9050-4a9c900bb38d
⛔ Files ignored due to path filters (2)
packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (4)
packages/core/src/Layout.tsxpackages/core/src/pages/[slug]/p.tsxpackages/core/src/sdk/analytics/hooks/useStartSession.tspackages/core/test/sdk/analytics/useStartSession.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/Layout.tsx
- Centralize campaign-type taxonomy in a single map in vrn.ts and make getTypeFromVrn total (returns null instead of throwing during render). - Extract cookie/userId orchestration into a useRecommendationUserId hook. - Use the canonical useScreenResize hook for paging instead of a bespoke checkIsMobile helper. - Unify the two ad-hoc retry mechanisms into a single src/utils/retry util (with backoff and AbortSignal support). - Document why the recommendations query fetches the full product shape (mapProductToProductCard is overridable).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/core/src/components/RecommendationShelf/useRecommendationUserId.ts (1)
23-46: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
campaignVrnonly feeds an error log but drives the effect.The cookie lookup doesn't depend on
campaignVrnfor its result — it's used solely in thecatchlog. Re-running the whole retry loop whenevercampaignVrnchanges is wasteful. If the log context matters, keep it but consider an empty dep array (cookie resolution is global), or readcampaignVrnvia a ref to avoid restarting the retry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/useRecommendationUserId.ts` around lines 23 - 46, The useEffect in useRecommendationUserId is being re-run unnecessarily because campaignVrn is only used in the catch log and does not affect getUserIdFromCookie or retry behavior. Update the dependency handling so the cookie lookup runs once for this global value, either by using an empty dependency array or by moving campaignVrn into a ref if you still need it in the error log. Keep the existing AbortController cleanup and the setUserId logic intact.packages/core/src/utils/retry.ts (1)
53-66: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAbort doesn't interrupt an in-flight delay.
When
signalaborts during thesetTimeoutwait, the loop only notices after the fulldelayelapses (up tomaxDelayMs). For the unmount case inuseRecommendationUserId, this leaves a pending timer running after cleanup. Consider wiring the timeout to the signal so it resolves early.♻️ Resolve the delay early on abort
- await new Promise((resolve) => setTimeout(resolve, delay)) - - if (signal?.aborted) { - break - } + await new Promise<void>((resolve) => { + const timer = setTimeout(resolve, delay) + signal?.addEventListener( + 'abort', + () => { + clearTimeout(timer) + resolve() + }, + { once: true } + ) + }) + + if (signal?.aborted) { + break + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/utils/retry.ts` around lines 53 - 66, The retry delay in `retry` does not stop early when the abort `signal` fires, so the pending `setTimeout` keeps running until the full delay completes. Update the wait inside `retry` to be abort-aware by wiring the timer to `signal` (or using an abortable promise) so the sleep resolves/rejects immediately on abort, then keep the existing `signal?.aborted` guard before the next `fn()` call.packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlias
@faststore/uicomponents with theUIprefix.This component file imports and renders
ProductShelf/Carouseldirectly from@faststore/ui; the repo convention asks these imports to useUIaliases.♻️ Proposed fix
-import { ProductShelf, Carousel } from '`@faststore/ui`' +import { ProductShelf as UIProductShelf, Carousel as UICarousel } from '`@faststore/ui`'- <ProductShelf> - <Carousel + <UIProductShelf> + <UICarousel id={id} itemsPerPage={itemsPerPage} variant="scroll" infiniteMode={false} {...carouselProps} > {items.map((item, index) => ( <div key={item.productId} className={styles.recommendationShelfItem} @@ /> </div> ))} - </Carousel> - </ProductShelf> + </UICarousel> + </UIProductShelf>As per coding guidelines, “Import from
@faststore/uiand alias withUIprefix to avoid local name collisions when composing library components.”Also applies to: 134-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx` at line 4, The RecommendationShelf component is importing and using `@faststore/ui` primitives without the required UI prefix alias. Update the imports in RecommendationShelf.tsx to alias ProductShelf and Carousel with the UI prefix, then adjust all usages in the RecommendationShelf render logic to reference those aliased names consistently. Keep the change local to the RecommendationShelf component and preserve existing behavior while following the repository convention.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsx`:
- Around line 126-127: The null-user test in
RecommendationShelf.browser.test.tsx is out of sync with the skip-fetch
contract. Update the test around useRecommendationUserId and
getRecommendationArguments to assert that useRecommendations receives null and
that the shelf renders empty when there is no resolved userId, unless the
component contract is intentionally changed to fetch without a user ID.
---
Nitpick comments:
In `@packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx`:
- Line 4: The RecommendationShelf component is importing and using `@faststore/ui`
primitives without the required UI prefix alias. Update the imports in
RecommendationShelf.tsx to alias ProductShelf and Carousel with the UI prefix,
then adjust all usages in the RecommendationShelf render logic to reference
those aliased names consistently. Keep the change local to the
RecommendationShelf component and preserve existing behavior while following the
repository convention.
In `@packages/core/src/components/RecommendationShelf/useRecommendationUserId.ts`:
- Around line 23-46: The useEffect in useRecommendationUserId is being re-run
unnecessarily because campaignVrn is only used in the catch log and does not
affect getUserIdFromCookie or retry behavior. Update the dependency handling so
the cookie lookup runs once for this global value, either by using an empty
dependency array or by moving campaignVrn into a ref if you still need it in the
error log. Keep the existing AbortController cleanup and the setUserId logic
intact.
In `@packages/core/src/utils/retry.ts`:
- Around line 53-66: The retry delay in `retry` does not stop early when the
abort `signal` fires, so the pending `setTimeout` keeps running until the full
delay completes. Update the wait inside `retry` to be abort-aware by wiring the
timer to `signal` (or using an abortable promise) so the sleep resolves/rejects
immediately on abort, then keep the existing `signal?.aborted` guard before the
next `fn()` call.
🪄 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: 4529d040-ee71-490e-a04b-261cdf683bb6
📒 Files selected for processing (11)
packages/core/src/components/RecommendationShelf/RecommendationShelf.tsxpackages/core/src/components/RecommendationShelf/useRecommendationUserId.tspackages/core/src/components/RecommendationShelf/useRecommendations.tspackages/core/src/components/RecommendationShelf/vrn.tspackages/core/src/sdk/analytics/hooks/useStartSession.tspackages/core/src/sdk/analytics/utils.tspackages/core/src/utils/retry.tspackages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsxpackages/core/test/components/RecommendationShelf/vrn.test.tspackages/core/test/sdk/analytics/utils.test.tspackages/core/test/utils/retry.test.ts
💤 Files with no reviewable changes (1)
- packages/core/src/sdk/analytics/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/sdk/analytics/hooks/useStartSession.ts
- packages/core/src/components/RecommendationShelf/useRecommendations.ts
| "type": "string", | ||
| "pattern": "^vrn:recommendations:[^:]+:(rec-cross-v2|rec-similar-v2|rec-persona-v2|rec-last-v2|rec-top-items-v2|rec-search-v2|rec-next-v2|rec-visual-v2):[^:]+$" | ||
| }, | ||
| "carouselConfiguration": { |
There was a problem hiding this comment.
One thing that is missing is the itemContext: https://github.com/vtex-apps/recommendation-shelf/blob/master/react/RecommendationShelf.tsx#L93
So the user can select wether we use the pdp product as reference or the cart items
| } | ||
| : {})} | ||
| > | ||
| <ProductCard |
There was a problem hiding this comment.
What about receiving a custom ProductCard?
| * - `null`: lookup finished without a usable id (e.g. the pixel never set it); | ||
| * - `string`: the resolved user id. | ||
| */ | ||
| export function useRecommendationUserId( |
| * Starts the anonymous recommendation/personalization session once per browser | ||
| * session. | ||
| * | ||
| * Mounted globally from `Layout` so it runs on every page, regardless of |
There was a problem hiding this comment.
We need to discuss this FastStore team. Since recommendations is an optional feature, we might need to either use the GlobalTracker approach or hide this under a feature flag. We don't want to receive requests from Accounts that are not active in our side
| usePageViewEvent(props) | ||
| // Implemented here because the personalization session must be initiated once | ||
| // per browser session on each page. | ||
| useStartSession() |
| """ | ||
| Returns personalized product recommendations for a given campaign. | ||
| """ | ||
| recommendations( |
There was a problem hiding this comment.
Where is the implementation for this recommendations query?
…n and update related logic - Updated GraphQL schema and mutation definitions to reflect the renaming of `startSession` to `startRecommendationSession`. - Adjusted integration tests and hooks to utilize the new mutation name. - Introduced a new hook, `useStartRecommendationSession`, to manage the session initiation process. - Updated the `Layout` component to call the new hook, ensuring the recommendation session starts correctly per browser session.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/sdk/analytics/hooks/useStartRecommendationSession.ts (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the empty-object assertion.
Line 42 does not need
as StartRecommendationSessionVariables;{}already satisfiesRecord<string, never>, so the cast only suppresses type checking in a file where we explicitly want to avoid assertions when possible. As per path instructions, "Ensure type safety and avoid type assertions when possible."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/sdk/analytics/hooks/useStartRecommendationSession.ts` at line 42, Remove the empty-object type assertion in useStartRecommendationSession so the default variables value is passed as a plain empty object instead of being cast to StartRecommendationSessionVariables. Update the useStartRecommendationSession hook to rely on the inferred Record<string, never> shape and keep type safety without suppressing checks.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/sdk/analytics/hooks/useStartRecommendationSession.ts`:
- Around line 61-68: The retry in useStartRecommendationSession only handles
undefined results, so transient request rejections from
runStartRecommendationSession exit immediately instead of backing off. Update
the retry path around retry(...) and runStartRecommendationSession so thrown
request/GraphQL errors are treated as retryable (for example by catching and
returning a retryable failure or otherwise preserving the retry loop), and make
sure the discarded void promise does not leave an unhandled rejection when the
endpoint is not ready.
---
Nitpick comments:
In `@packages/core/src/sdk/analytics/hooks/useStartRecommendationSession.ts`:
- Line 42: Remove the empty-object type assertion in
useStartRecommendationSession so the default variables value is passed as a
plain empty object instead of being cast to StartRecommendationSessionVariables.
Update the useStartRecommendationSession hook to rely on the inferred
Record<string, never> shape and keep type safety without suppressing checks.
🪄 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: bf4f57b5-c8c2-42c0-ad4d-cc0a7d238c59
⛔ Files ignored due to path filters (3)
packages/api/src/__generated__/schema.tsis excluded by!**/__generated__/**and included bypackages/**packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (8)
packages/api/src/platforms/vtex/typeDefs/mutation.graphqlpackages/api/src/platforms/vtex/typeDefs/recommendation.graphqlpackages/api/test/integration/schema.test.tspackages/core/src/Layout.tsxpackages/core/src/sdk/analytics/hooks/useStartRecommendationSession.tspackages/core/src/sdk/analytics/hooks/useStartSession.tspackages/core/test/sdk/analytics/useStartRecommendationSession.test.tspackages/core/test/server/index.test.ts
💤 Files with no reviewable changes (1)
- packages/api/src/platforms/vtex/typeDefs/recommendation.graphql
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/sdk/analytics/hooks/useStartSession.ts
- packages/core/src/Layout.tsx
…ent interface Resolve recommendations in @faststore/api (recommendations query and startRecommendationSession mutation) and map the BFF payload straight to the normalized StoreProduct shape via pickBestSku + enhanceSku, so recommendation shelves expose the same product interface as the rest of the storefront. Update RecommendationShelf and useRecommendations to consume StoreProduct directly, with a configurable product card component and mapper, and drop the now-unused RecommendationProduct mapper.
…nResult and update related logic - Renamed `startSession` to `startRecommendationSession` in the recommendation client and updated the corresponding types. - Adjusted the resolver to call the new session initiation method. - Removed the now-unnecessary `useStartSession` hook to streamline the session management process.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/components/RecommendationShelf/useRecommendations.ts (1)
42-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the forced
RecommendationInputcast.
args ?? ({} as RecommendationInput)sidesteps the type-safety rule for this path and makes this hook easier to break later if the skip condition changes. A nullableuseQueryvariables type keeps the call safe without an assertion. As per path instructions, TypeScript files should “Ensure type safety and avoid type assertions when possible.”Possible cleanup
export const useRecommendations = (args: RecommendationInput | null) => { const { data, isLoading, error } = useQuery< FetchRecommendationsQueryQuery, - RecommendationInput - >(query, args ?? ({} as RecommendationInput), { + RecommendationInput | null + >(query, args, { doNotRun: args === null, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/RecommendationShelf/useRecommendations.ts` around lines 42 - 48, The useRecommendations hook is forcing a RecommendationInput assertion with args ?? ({} as RecommendationInput), which bypasses type safety and can break if the doNotRun logic changes. Update the useQuery call in useRecommendations to accept nullable variables instead of asserting a non-null RecommendationInput, and pass args directly so the hook remains type-safe without a cast.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/platforms/vtex/clients/recommendation/index.ts`:
- Around line 21-24: The Recommendation client is using an Options type that is
not declared in this module, so update the VTEX recommendation client to import
and reuse the shared client options type used by the other VTEX client
factories. Locate the Recommendation function signature and replace the
unresolved Options reference with the shared type import so the module compiles
consistently with the other client implementations.
In `@packages/api/src/platforms/vtex/resolvers/recommendations.ts`:
- Around line 26-32: Validate the inputs in the VTEX recommendations resolver
before calling the platform client. In the recommendations resolver, reject
empty or malformed values for campaignVrn, userId, and products instead of
forwarding them as-is, and surface a GraphQL validation error locally. Add
server-side checks around the recommendations() call so only clean, well-formed
payloads reach recommendation.recommendations, while keeping locale and
salesChannel handling unchanged.
In `@packages/api/src/platforms/vtex/resolvers/startRecommendationSession.ts`:
- Around line 16-18: The mutation currently reports success unconditionally even
when `ctx.clients.recommendation.startSession()` returns `undefined`, so update
`startRecommendationSession` to validate the `StartSessionResult` before
returning. Use the return type from `RecommendationClient.startSession` and the
success payload shape from `StartSessionResult` to detect missing session data,
and only return true when a session is actually created; otherwise surface an
appropriate GraphQL error so the caller can retry.
---
Nitpick comments:
In `@packages/core/src/components/RecommendationShelf/useRecommendations.ts`:
- Around line 42-48: The useRecommendations hook is forcing a
RecommendationInput assertion with args ?? ({} as RecommendationInput), which
bypasses type safety and can break if the doNotRun logic changes. Update the
useQuery call in useRecommendations to accept nullable variables instead of
asserting a non-null RecommendationInput, and pass args directly so the hook
remains type-safe without a cast.
🪄 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: 68c32c2d-c870-4e19-8407-5d65c32ecc7c
⛔ Files ignored due to path filters (3)
packages/api/src/__generated__/schema.tsis excluded by!**/__generated__/**and included bypackages/**packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (12)
packages/api/src/platforms/vtex/clients/index.tspackages/api/src/platforms/vtex/clients/recommendation/index.tspackages/api/src/platforms/vtex/clients/recommendation/types/RecommendationResult.tspackages/api/src/platforms/vtex/resolvers/mutation.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/resolvers/recommendations.tspackages/api/src/platforms/vtex/resolvers/startRecommendationSession.tspackages/api/src/platforms/vtex/typeDefs/recommendation.graphqlpackages/core/src/components/RecommendationShelf/RecommendationShelf.tsxpackages/core/src/components/RecommendationShelf/RecommendationShelf.types.tspackages/core/src/components/RecommendationShelf/useRecommendations.tspackages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/api/src/platforms/vtex/clients/recommendation/types/RecommendationResult.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/components/RecommendationShelf/RecommendationShelf.tsx
- packages/core/test/components/RecommendationShelf/RecommendationShelf.browser.test.tsx
| export const Recommendation = ( | ||
| { account, environment }: Options, | ||
| ctx: GraphqlContext | ||
| ) => { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `Options` is declared globally or should be imported from a shared VTEX module.
rg -n --type=ts '\b(interface|type)\s+Options\b|\bdeclare global\b' packages/api/src/platforms/vtex
sed -n '1,40p' packages/api/src/platforms/vtex/clients/recommendation/index.ts
fd -i 'index.ts' packages/api/src/platforms/vtex/clientsRepository: vtex/faststore
Length of output: 1610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n-- recommendation index --\n'
sed -n '1,220p' packages/api/src/platforms/vtex/clients/recommendation/index.ts
printf '\n-- search index --\n'
sed -n '1,220p' packages/api/src/platforms/vtex/clients/search/index.ts
printf '\n-- search ProductSearchResult types --\n'
sed -n '1,220p' packages/api/src/platforms/vtex/clients/search/types/ProductSearchResult.ts
printf '\n-- all Option type declarations under vtex clients --\n'
rg -n --type=ts '\b(interface|type)\s+Options\b' packages/api/src/platforms/vtex/clientsRepository: vtex/faststore
Length of output: 12176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n-- all exported Options types in repo --\n'
rg -n --type=ts 'export\s+(interface|type)\s+Options\b' packages
printf '\n-- all imports of Options in vtex clients --\n'
rg -n --type=ts "import type \{[^}]*\bOptions\b[^}]*\}" packages/api/src/platforms/vtex/clients packages/api/src/platforms/vtex
printf '\n-- recommendation client neighbors --\n'
sed -n '1,220p' packages/api/src/platforms/vtex/clients/index.ts
sed -n '1,220p' packages/api/src/platforms/vtex/clients/commerce/index.tsRepository: vtex/faststore
Length of output: 302
Import the shared VTEX client options type.
Options is referenced here but never imported or declared in this module, so TypeScript will fail to compile this file. Reuse the same client options type used by the other VTEX clients.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/platforms/vtex/clients/recommendation/index.ts` around lines
21 - 24, The Recommendation client is using an Options type that is not declared
in this module, so update the VTEX recommendation client to import and reuse the
shared client options type used by the other VTEX client factories. Locate the
Recommendation function signature and replace the unresolved Options reference
with the shared type import so the module compiles consistently with the other
client implementations.
…xt and carousel settings - Introduced `itemsContext` prop to allow selection between 'PDP' and 'CART' for product context in recommendations. - Updated `RecommendationShelf` to utilize the new `itemsContext` for fetching recommendations based on the selected context. - Refactored carousel configuration to directly accept `itemsPerPageDesktop`, `itemsPerPageMobile`, `variant`, `infiniteMode`, and `controls` as props, improving customization options. - Adjusted related types and documentation to reflect these changes.
renatomaurovtex
left a comment
renatomaurovtex
left a comment
There was a problem hiding this comment.
Review — feat(core): add RecommendationShelf section with personalization tracking
Reviewed the api client/resolvers (recommendation, recommendations, startRecommendationSession), the core component + hooks (RecommendationShelf, useRecommendations, useRecommendationUserId, useStartRecommendationSession, vrn, retry) and the p.tsx/Layout wiring. Solid, well-tested feature (browser test + unit tests for vrn/retry/useRecommendations/utils). Boundaries are clean: BFF returns the IS Product shape so pickBestSku+enhanceSku reuse is correct and avoids a second search round-trip; recommendations is @cacheControl(scope:"private") which is right for personalized output; error logging deliberately omits userId. Good.
🟠 Meta-tag overlap with open PR #3383 (same author)
p.tsx adds product:id/sku/name/category/url/brand/condition to additionalMetaTags. Open PR #3383 ("pdp product SKU meta tag", also yours) adds product:sku/name/category/url to the same array. If both land on dev you'll get duplicate product:sku/product:name/etc. meta tags. This PR is a superset — recommend closing/rebasing #3383 onto this, or dropping the tag block from whichever merges second. Please de-conflict before merge.
🟡 Non-personalized campaigns are gated on the pixel cookie
getRecommendationArguments returns null whenever !userId, so TOP_ITEMS and SEARCH_BASED campaigns — which don't conceptually need an anonymous shopper id — won't render at all if the personalization pixel never sets vtex-rec-user-id (ad-blockers, consent-gated cookies, slow pixel beyond the retry budget). Consider allowing those campaign types to fetch with an empty userId so they degrade to non-personalized results instead of a hidden shelf.
🟡 usePDP() in a globally-mounted component
RecommendationShelf is registered as a CMS global component and calls usePDP() directly, yet the PR body notes usePDP() "has no context above PageProvider" (which is why useStartRecommendationSession reads page props instead). Please confirm usePDP() returns { data: undefined } safely (rather than throwing) when the shelf is placed on a non-PDP page — the productDetailPage?.product... optional chaining suggests it's safe, but worth verifying given the body's caveat.
💬 Minor / nits
shouldAddAFAttrchecksproductIds.lengthwhereproductIdsis a comma-joined string; it works (empty when no items) but reads like an array-length check —items.length > 0would be clearer.- Several CodeRabbit "Major" comments reference
queries/request.ts/queries/useLazyQuery.tsthat no longer exist in the diff — looks like you refactored to the coreuseQuery/useLazyQuery, which resolves their "variables in GET query string" privacy concern. Worth confirming the coreuseQuerypath doesn't surfaceuserIdin a CDN-cached GET URL for thisscope:privateop. retryreturns the last value even on budget exhaustion (documented) — callers handle the give-up case correctly.
CI: FastStore build/test, codesandbox, node-ci green. SonarQube red (check new-code coverage/quality gate on the new files); UI Tests pending (Chromatic baselines).
Verdict: Approved with comments. No hard blockers, but please de-conflict the product:* meta tags with #3383 before merge and confirm the usePDP()-on-non-PDP safety.
| const headers: HeadersInit = withCookie({ | ||
| accept: 'application/json', | ||
| 'content-type': 'application/json', | ||
| 'x-vtex-rec-origin': `${account}/${REC_ORIGIN_SUFFIX}`, |
There was a problem hiding this comment.
I believe it is best to forward all the cookies, otherwise you will need to open the segment and parse all the infos.
There was a problem hiding this comment.
Good point — that's already what we do here. The client forwards all cookies through getWithCookie/withCookie: it sets the cookie header to the full normalized ctx.headers.cookie (including vtex_segment, checkout.vtex.com, etc.) merged with any updates from ctx.storage.cookies. So the BFF receives the whole segment and we never open/parse it here.
The salesChannel/locale query params come pre-resolved from ctx.storage.channel/ctx.storage.locale (parsed once upstream by FastStore, not re-parsed in this client), so they're just a convenience on top of the forwarded cookies. Happy to drop them and rely solely on the forwarded vtex_segment if we're confident the recommend-bff reads them from the cookie — let me know and I'll simplify.
| "enum": ["PDP", "CART"], | ||
| "enumNames": ["Product page", "Cart"] | ||
| }, | ||
| "itemsPerPageDesktop": { |
There was a problem hiding this comment.
Since we are segregating the product card configuration into its own object I would sugest doing the same here for the carrousel, wdyt?
| controls, | ||
| productCardConfiguration, | ||
| }: RecommendationShelfProps<TCardProps>) { | ||
| const CardComponent = (ProductCard ?? |
| usePageViewEvent(props) | ||
| // Implemented here because the personalization session must be initiated once | ||
| // per browser session on each page. | ||
| useStartRecommendationSession() |
There was a problem hiding this comment.
Again, how do we disable this for stores that are not using Recommendations?
There was a problem hiding this comment.
Good question.
Should all the accounts have the recommendation feature and have requests made to FS API -> recommendations API?
Maybe we can introduce a new feature/experimental flag so the store can choose to enable or disable it.
There was a problem hiding this comment.
Agreed — this shouldn't run for stores that aren't using Recommendations. Following the same pattern we already use for other opt-in features, I suggest gating it behind an experimental flag in discovery.config so it's off by default and each store explicitly opts in.
There was a problem hiding this comment.
@eduardoformiga one question, if we do it via feature flag, how is it enabled? Can someone do it via admin or does it require a code change/deploy? Asking because one of our requirements is that a Growth person can enable and configure Recommendations in a VTEX store, so without code changes...
Co-authored-by: Felipe Sales <sales.felipeb@gmail.com>
There was a problem hiding this comment.
Não existe uma regra específica, mas penso que em vez de ser um novo client para recommendations, poderia ser um namespace dentro do vtex Commerce client. Você enxerga mais operações existentes nesse cliente além das adicionadas até o momento? o que achas?
There was a problem hiding this comment.
@thiagopereira-vtex essa acho que cabe mais como um namespace dentro do client de commerce que ja existe, ao invés de criar um novo client. O que achas?
| userId: String | ||
| products: [String!] | ||
| ): RecommendationResponse! | ||
| @cacheControl(scope: "private", sMaxAge: 120, staleWhileRevalidate: 3600) |
There was a problem hiding this comment.
A recomendação é feita pra cada usuário (userId)? Acho que, nesse caso, faz sentido ser private: ficar no browser do usuário e não "vazar" para cdn ou outra camada compartilhada. Caso contrário, seria melhor public.
There was a problem hiding this comment.
Can we rename it to the lowercase cms_component__recommendationshelf.jsonc? just to keep the pattern with the other files.
| "$extends": ["#/$defs/base-component"], | ||
| "$componentKey": "RecommendationShelf", | ||
| "$componentTitle": "RecommendationShelf", | ||
| "type": "object", |
There was a problem hiding this comment.
I miss the title and description here.
There was a problem hiding this comment.
would be good to regenerate it after changing the packages/core/cms/faststore/components/cms_component__RecommendationShelf.jsonc as well
There was a problem hiding this comment.
This section comes from CMS, right? What about moving all the RecommendationShelf files to the packages/core/src/components/sections folder?
| const VRN_TYPE_TO_RECOMMENDATION = { | ||
| 'rec-cross-v2': 'CROSS_SELL', | ||
| 'rec-similar-v2': 'SIMILAR_ITEMS', | ||
| 'rec-persona-v2': 'PERSONALIZED', | ||
| 'rec-last-v2': 'LAST_SEEN', | ||
| 'rec-top-items-v2': 'TOP_ITEMS', | ||
| 'rec-search-v2': 'SEARCH_BASED', | ||
| 'rec-next-v2': 'NEXT_INTERACTION', | ||
| 'rec-visual-v2': 'VISUAL_SIMILARITY', | ||
| } as const satisfies Record<string, RecommendationType> |
There was a problem hiding this comment.
By any chance, could this list be expanded, and would it be better to get it from an API instead of adding it here manually?
There was a problem hiding this comment.
Thanks for the suggestion! We don't have this on our roadmap for now, so I'd rather keep it out of the scope of this PR. @salesfelipe what do you think — do you see this as something we should prioritize later?
There was a problem hiding this comment.
That is something that rarely changes, I would rather keep it like this, isn't worth addding a new network call that could make u slower to render...
| usePageViewEvent(props) | ||
| // Implemented here because the personalization session must be initiated once | ||
| // per browser session on each page. | ||
| useStartRecommendationSession() |
There was a problem hiding this comment.
Good question.
Should all the accounts have the recommendation feature and have requests made to FS API -> recommendations API?
Maybe we can introduce a new feature/experimental flag so the store can choose to enable or disable it.
| }, | ||
| { | ||
| property: 'product:condition', | ||
| content: 'new', |
There was a problem hiding this comment.
Is it important to the PR to add all these new seo additionalMetaTags?
There was a problem hiding this comment.
For this PR, the only essential ones are product:id and product:sku — those are what we actually rely on here. The remaining product:* tags aren't required, they're complementary and improve SEO / social previews.
… and enable opt-in feature - Added server-side validation for `campaignVrn`, `userId`, and `products` in the recommendations resolver to ensure clean input. - Introduced an opt-in flag `enableRecommendations` in the discovery configuration to control the initiation of recommendation sessions and API calls. - Updated `RecommendationShelf` and related components to respect the opt-in flag, preventing unnecessary API calls when recommendations are disabled. - Enhanced tests to cover scenarios for the new validation and opt-in feature.
…t recommendations - Introduced the `RecommendationShelf` component to display a carousel of personalized product recommendations based on a VTEX Recommendations campaign. - Implemented configuration options for carousel behavior, including items per page for desktop and mobile, navigation controls, and infinite scrolling. - Added types and hooks to manage recommendation fetching and user identification. - Created associated styles and tests to ensure functionality and maintainability. - Updated existing components and tests to integrate the new `RecommendationShelf` structure.
…te handling - Changed the prop type for `RecommendationShelf` from `RecommendationShelfProps<TCardProps>` to `Readonly<RecommendationShelfProps<TCardProps>>` for better type safety. - Modified the empty state return from an empty fragment to `null` for clearer intent and improved rendering performance.
| // response, so the shelf renders identical product cards to regular shelves. | ||
| // We select `...ProductSummary_product` (the fragment consumed by `ProductCard`) | ||
| // to keep this interface consistent with the rest of the components. | ||
| const query = gql(`query FetchRecommendationsQuery( |
There was a problem hiding this comment.
Just to keep the pattern: Server side queries starts with Server, Client side queries starts with Client. We'll need to regenerate the schemas afterward.
| const query = gql(`query FetchRecommendationsQuery( | |
| const query = gql(`query ClientRecommendationsQuery( |
There was a problem hiding this comment.
This file is still out of the pattern.
- Section SCSS files are named
section.module.scss. - The root class should be named
section. - Maybe we can use local tokens and data attributes for internal classes/styles. reference
https://developers.vtex.com/docs/guides/faststore/styling-data-attributes
https://developers.vtex.com/docs/guides/faststore/using-themes-components
| [getComponentKey(RecommendationShelf, 'RecommendationShelf')]: | ||
| RecommendationShelf, |
There was a problem hiding this comment.
@thiagopereira-vtex Looks like this section should be added to PDP/PLPs instead of the global one, right?
There was a problem hiding this comment.
@thiagopereira-vtex essa acho que cabe mais como um namespace dentro do client de commerce que ja existe, ao invés de criar um novo client. O que achas?
- Fold the recommendation client into the VtexCommerce client as a `commerce.recommendation` namespace (same account host) and drop the standalone client. - Rename the client query to `ClientRecommendationsQuery` to follow the client/server prefix convention and regenerate the GraphQL types. - Align the shelf styles with the section pattern: `section.module.scss`, `.section` root class and a `data-fs-recommendation-shelf-item` attribute.
…S6759 - Add unit tests for the `recommendations` and `startRecommendationSession` resolvers, the `commerce.recommendation` client namespace and the `useRecommendationUserId` hook to raise new-code coverage above the gate. - Mark `RecommendationShelfProps` members as `readonly` (instead of the `Readonly<>` wrapper, which Sonar S6759 doesn't detect on a generic component) to satisfy the read-only props rule.
Replaces the `(value) => Boolean(value)` wrapper flagged by Sonar with a direct `Boolean` reference.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/platforms/vtex/clients/commerce/index.ts`:
- Around line 1119-1148: The recommendations() call is not forwarding BFF
Set-Cookie headers because it invokes fetchAPI with only headers and never
passes the storeCookies callback used by startRecommendationSession(). Update
recommendations() in the commerce client to accept and forward the same
cookie-storage hook (via fetchAPI’s third argument or equivalent) so vtex-rec-*
cookies are persisted through ctx.storage.cookies instead of being dropped.
In
`@packages/core/src/components/sections/RecommendationShelf/RecommendationShelf.tsx`:
- Around line 182-184: The RecommendationShelf heading can become empty when
both title and data?.campaign.title are missing, so update the h2 in
RecommendationShelf to always render accessible text or an ARIA label. Adjust
the title fallback logic in RecommendationShelf.tsx so the heading uses a
non-empty fallback string (or equivalent accessible labeling) instead of
rendering nothing, keeping the semantic h2 while preventing an empty
announcement for screen readers.
- Around line 100-121: The PDP fallback in RecommendationShelf is still unsafe
because productDetailPage?.product.isVariantOf.productGroupID can throw when
data.product is missing. Update the contextProducts useMemo in
RecommendationShelf.tsx to guard each nested access with optional chaining all
the way through product, isVariantOf, and productGroupID, so the fallback
returns an empty array when PDP data is incomplete.
🪄 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: 4a96312d-a524-4353-89e0-14117c91ab80
⛔ Files ignored due to path filters (3)
packages/api/src/__generated__/schema.tsis excluded by!**/__generated__/**and included bypackages/**packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**
📒 Files selected for processing (26)
packages/api/src/platforms/vtex/clients/commerce/index.tspackages/api/src/platforms/vtex/clients/commerce/types/RecommendationResult.tspackages/api/src/platforms/vtex/resolvers/mutation.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/resolvers/recommendations.tspackages/api/src/platforms/vtex/resolvers/startRecommendationSession.tspackages/api/src/platforms/vtex/typeDefs/query.graphqlpackages/api/src/platforms/vtex/typeDefs/recommendation.graphqlpackages/api/test/integration/schema.test.tspackages/api/test/unit/platforms/vtex/clients/commerce.test.tspackages/api/test/unit/platforms/vtex/resolvers/recommendations.test.tspackages/api/test/unit/platforms/vtex/resolvers/startRecommendationSession.test.tspackages/core/cms/faststore/components/cms_component__recommendationshelf.jsoncpackages/core/cms/faststore/schema.jsonpackages/core/discovery.config.default.jspackages/core/src/Layout.tsxpackages/core/src/components/cms/global/Components.tspackages/core/src/components/sections/RecommendationShelf/RecommendationShelf.tsxpackages/core/src/components/sections/RecommendationShelf/RecommendationShelf.types.tspackages/core/src/components/sections/RecommendationShelf/index.tspackages/core/src/components/sections/RecommendationShelf/section.module.scsspackages/core/src/components/sections/RecommendationShelf/useRecommendationUserId.tspackages/core/src/components/sections/RecommendationShelf/useRecommendations.tspackages/core/src/components/sections/RecommendationShelf/vrn.tspackages/core/src/pages/[slug]/p.tsxpackages/core/src/sdk/analytics/hooks/useStartRecommendationSession.ts
💤 Files with no reviewable changes (3)
- packages/core/src/components/sections/RecommendationShelf/index.ts
- packages/core/src/components/sections/RecommendationShelf/useRecommendationUserId.ts
- packages/core/src/components/sections/RecommendationShelf/vrn.ts
✅ Files skipped from review due to trivial changes (1)
- packages/api/src/platforms/vtex/clients/commerce/types/RecommendationResult.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/api/test/integration/schema.test.ts
- packages/core/src/Layout.tsx
- packages/core/src/pages/[slug]/p.tsx
- packages/core/src/sdk/analytics/hooks/useStartRecommendationSession.ts
- packages/api/src/platforms/vtex/typeDefs/recommendation.graphql
- packages/core/src/components/cms/global/Components.ts
- packages/api/src/platforms/vtex/resolvers/mutation.ts
- packages/api/src/platforms/vtex/typeDefs/query.graphql
- packages/core/cms/faststore/schema.json
| <h2 className="text__title-section layout__content"> | ||
| {title ?? data?.campaign.title} | ||
| </h2> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Heading can render empty when neither title nor campaign.title exist.
<h2>{title ?? data?.campaign.title}</h2> — if both are undefined, the heading renders with no text content, which is a minor a11y regression (screen readers announce an empty heading). As per path instructions, packages/**/src/components/** component code should "Use semantic HTML and proper ARIA labels for accessibility".
♿ Proposed fallback
- <h2 className="text__title-section layout__content">
- {title ?? data?.campaign.title}
- </h2>
+ {(title ?? data?.campaign.title) && (
+ <h2 className="text__title-section layout__content">
+ {title ?? data?.campaign.title}
+ </h2>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <h2 className="text__title-section layout__content"> | |
| {title ?? data?.campaign.title} | |
| </h2> | |
| {(title ?? data?.campaign.title) && ( | |
| <h2 className="text__title-section layout__content"> | |
| {title ?? data?.campaign.title} | |
| </h2> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/core/src/components/sections/RecommendationShelf/RecommendationShelf.tsx`
around lines 182 - 184, The RecommendationShelf heading can become empty when
both title and data?.campaign.title are missing, so update the h2 in
RecommendationShelf to always render accessible text or an ARIA label. Adjust
the title fallback logic in RecommendationShelf.tsx so the heading uses a
non-empty fallback string (or equivalent accessible labeling) instead of
rendering nothing, keeping the semantic h2 while preventing an empty
announcement for screen readers.
Source: Path instructions
- Updated the VtexCommerce client to include the `storeCookies` option in the fetchAPI call for recommendations. - Enhanced unit tests to verify the presence of the `storeCookies` property in the API call. - Refactored the RecommendationShelf component to ensure safe access to product group ID.
|
0 New Issues
0 Fixed Issues
0 Accepted Issues4 participants
What's the purpose of this pull request?
Adds a CMS-configurable RecommendationShelf section that renders VTEX personalization recommendations, and wires up the anonymous personalization session/tracking globally.
How it works?
RecommendationShelfsection: fetches recommendations for a givencampaignVrn(resolving the campaign type from the VRN) and renders them with the shared coreProductCard. AmapRecommendationToProductCardadapter maps the VTEXRecommendationProductinto theProductSummary_ProductFragmentshape, so recommendation cards look identical to regular shelves.useStartSession(insrc/sdk/analytics/hooks): mounted globally fromLayout, it starts the anonymous personalization session once per browser session (guarded by a cookie, with backoff) and reports a product view on PDPs. The product is derived from the page props (same pattern asusePageViewEvent) becauseusePDP()has no context abovePageProvider.How to test it?
RecommendationShelfsection via the CMS with a validcampaignVrn.StartSessionfires once per browser session andSendProductViewEventfires on PDPs.References
N/A
Summary by CodeRabbit
New Features
Bug Fixes