Feature/medusa integration - product section#662
Conversation
- Refactored `subtotal` field to remove redundant `label` attribute. - Enhanced MedusaJS product queries to include `product.images` and filter by published status.
…hance query parameters
…or enhanced query control
|
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 end-to-end product variant support (SDK, service, API, mappers, CMS, frontend variant selector and routing), flattens order-list subtotal shape, exposes product Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Client as ProductDetails Client
participant Router as Next Router
participant Server as ProductDetails Server
participant SDK as Blocks SDK
participant API as Backend API
User->>Client: Select variant from dropdown
Client->>Client: compute URL with variantSlug
Client->>Router: router.push(new URL)
Router->>Server: Request page with variantSlug
Server->>SDK: sdk.blocks.getProductDetails({ id, variantSlug })
SDK->>API: GET /products/{id} or /products/{id}/{variantSlug}
API-->>SDK: Return product details (variant-aware)
SDK-->>Server: Return ProductDetailsBlock
Server-->>Client: Rendered page with variant-specific data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/blocks/recommended-products/src/api-harmonization/recommended-products.service.ts (1)
28-55:⚠️ Potential issue | 🟡 MinorHardcoded
limit: 7is fragile — post-fetch filtering can yield fewer or more than 6 results.The comment on Line 30 assumes exactly one product is removed (the excluded product), but the image filter on Line 42 can remove additional products, leaving fewer than 6. Conversely, when
query.excludeProductIdis unset, all 7 may pass through.Consider either:
- Over-fetching with a larger limit and slicing after filtering, or
- Adding a
.slice(0, desiredCount)after filtering to guarantee a consistent count.Proposed fix: slice after filtering to guarantee count
+ const RECOMMENDED_LIMIT = 6; + const filteredProducts: Model.ProductSummary[] = products.data .filter((product: Products.Model.Product) => { if (query.excludeProductId && product.id === query.excludeProductId) { return false; } return product.image; }) + .slice(0, RECOMMENDED_LIMIT) .map((product: Products.Model.Product) => ({Also consider increasing the fetch limit to give more headroom for filtered-out products:
- limit: 7, // Fetch 7 to have 6 after excluding current product + limit: 12, // Over-fetch to ensure 6 remain after filteringpackages/integrations/mocked/src/modules/products/products.mapper.ts (1)
273-323:⚠️ Potential issue | 🟠 MajorApply variant overrides to related products for consistency with
mapProducts
mapProductsapplies variant overrides (selecting the first variant and updatingvariantIdandlink), butmapRelatedProductsdoes not. When a product with variants appears in the related products list, it will be missing the variant selection, causing inconsistent behavior between the product list and related products views.Align
mapRelatedProductswithmapProductsby adding the same variant override logic before returning the data.
🤖 Fix all issues with AI agents
In `@apps/frontend/next.config.ts`:
- Around line 41-44: Remove the IANA placeholder by deleting the remotePatterns
entry that lists hostname: 'example.com' (the snippet defining protocol:
'https', hostname: 'example.com') from the Next.js image configuration; update
the remotePatterns array used in your exported Next.js config (e.g., the
remotePatterns property on the exported config object) to only include real,
intended hostnames or leave it empty if no external images are needed, and
ensure no leftover references to 'example.com' remain.
In `@packages/blocks/product-details/src/frontend/ProductDetails.client.tsx`:
- Around line 37-44: The handleVariantChange implementation is fragile because
it uses endsWith to strip currentVariantSlug; instead, parse product.link into
path segments (split by '/'), replace the last segment when it equals
currentVariantSlug (or always replace the last segment if you guarantee the last
segment is the variant), then rejoin and call router.push with the new path;
update the logic in handleVariantChange to operate on segments rather than
substring slicing and keep references to product.link, currentVariantSlug, and
router.push to locate the change.
In `@packages/integrations/medusajs/src/modules/products/products.mapper.ts`:
- Around line 288-290: In mapRelatedProducts, replace using the variant-level id
(targetProduct.id) for the mapped object's id with the product-level id used
elsewhere so filtering like excludeProductId works; update the id assignment in
mapRelatedProducts to use targetProduct.product?.id || targetProduct.id
(matching mapProduct and mapProducts) so downstream logic receives the product
ID consistently.
- Around line 222-229: The client-side category filtering applied to products
(using categoryFilter and filtering data.products into products) breaks
pagination and total calculation; instead, pass the categoryFilter as
category_id to the server when calling Medusa (use AdminProductListParams with
category_id) so the API returns paginated results filtered server-side, stop
filtering data.products in the mapper (remove the products =
products.filter(...) block), and ensure total uses the server's count
(data.count) rather than products.length when category_id is used.
In `@packages/integrations/medusajs/src/modules/products/products.service.ts`:
- Around line 100-142: When getProductByHandle receives a variantSlug but no
matching variant is found (the block that currently falls back to
product.variants[0]), emit a warning so stale/incorrect URLs are visible: after
computing matchingVariant (in the getProductByHandle method), if variantSlug is
truthy and matchingVariant is undefined, call this.logger?.warn(...) or
console.warn(...) with context (handle, variantSlug, product.id and available
variant slugs via this.slugify over product.variants titles/options) before
continuing to fall back to product.variants[0]; keep the remaining flow (setting
variant and calling getVariant) unchanged and ensure the message is concise and
informative.
In
`@packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.product-list.mapper.ts`:
- Line 8: The DE and PL locale entries in the product list mapper are missing
the variantId query param causing variant selection to be lost; update the
detailsUrl property in cms.product-list.mapper.ts for the DE and PL mappings
(the detailsUrl fields at the indicated occurrences) to include
'?variantId={variantId}' (e.g., '/products/{id}?variantId={variantId}'),
matching the EN entry so variant-specific navigation works correctly.
In `@packages/integrations/mocked/src/modules/products/products.mapper.ts`:
- Around line 114-164: VARIANT_OVERRIDES_DE contains price.currency set to 'USD'
for the pro and max variants; update those price objects in VARIANT_OVERRIDES_DE
(keys "pro" and "max") to use 'EUR' instead of 'USD' so the German locale prices
are in euros; search VARIANT_OVERRIDES_DE for any other price.currency entries
and change them to 'EUR' if they also were incorrectly copied as 'USD'.
🧹 Nitpick comments (11)
packages/blocks/order-list/src/frontend/OrderList.client.stories.tsx (1)
265-268: Floating-point artifacts in story data values.Lines 266 and 410 contain unrounded floating-point values (
773.9459999999999,458.95500000000004). While this is only story/test data, these could surface as visual noise in Storybook rendering. Consider rounding to a reasonable precision (e.g., 2–4 decimal places).Proposed fix
subtotal: { - value: 773.9459999999999, + value: 773.946, currency: 'USD', },subtotal: { - value: 458.95500000000004, + value: 458.955, currency: 'USD', },Also applies to: 409-412
packages/integrations/mocked/src/modules/products/products.mapper.ts (1)
7-112: Consider extracting variant override data to a separate mock file.~160 lines of hardcoded variant overrides occupy most of this mapper file. Other mock data (e.g.,
MOCK_PRODUCTS_EN/PL/DE) is already imported fromproducts.mock.ts. Moving the variant overrides alongside would keep the mapper focused on transformation logic.packages/blocks/product-details/src/frontend/ProductDetails.client.tsx (2)
25-26:productIdis destructured but never used.The
productIdprop is extracted from props on line 25 but has no references in the component body. If it's not needed here, remove it from the destructuring to avoid confusion.
75-93: Duplicated variant selector markup.The variant
<Select>block is copy-pasted for the mobile (lines 75–93) and desktop (lines 193–211) layouts. Consider extracting it into a small local component to keep both instances in sync.Also applies to: 193-211
packages/blocks/product-details/src/api-harmonization/product-details.service.ts (1)
20-33: Naming mismatch:variantSlugis passed asvariantId.The parameter is named
variantSlug(line 20) but is forwarded asvariantId(line 32) toproductsService.getProduct. If the downstream service truly expects an ID, the parameter here should be named consistently (e.g.,variantId). If it's actually a slug used for lookup, the field name in thegetProductcall should reflect that. Consistent naming avoids confusion about what this value represents.packages/integrations/medusajs/src/modules/medusajs/medusajs.service.ts (1)
24-26: Config fallback to empty string is intentional but slightly misleading.
this.config.get('MEDUSAJS_BASE_URL') || ''converts bothundefinedand''to'', then the!this._medusaBaseUrlcheck below catches it. This works, but the double-negative pattern (default to empty, then reject empty) could be clearer by simply checking for the raw config value.♻️ Slightly clearer config retrieval
- this._medusaBaseUrl = this.config.get('MEDUSAJS_BASE_URL') || ''; - this._medusaPublishableApiKey = this.config.get('MEDUSAJS_PUBLISHABLE_API_KEY') || ''; - this._medusaAdminApiKey = this.config.get('MEDUSAJS_ADMIN_API_KEY') || ''; + this._medusaBaseUrl = this.config.get('MEDUSAJS_BASE_URL') ?? ''; + this._medusaPublishableApiKey = this.config.get('MEDUSAJS_PUBLISHABLE_API_KEY') ?? ''; + this._medusaAdminApiKey = this.config.get('MEDUSAJS_ADMIN_API_KEY') ?? '';packages/integrations/medusajs/src/modules/products/products.mapper.ts (2)
10-18: Duplicatedslugify— extract to a shared utility.This
slugifyimplementation is identical toProductsService.slugifyinproducts.service.ts(lines 144–151). Extract it into a shared utility module (e.g.,utils/slugify.ts) and import in both places.
166-170: Unnecessary optional chaining after null guard.Line 166 checks
if (!productVariant)and throws. Line 170 then usesproductVariant?.product— the?.is redundant sinceproductVariantis guaranteed non-nullish at this point.♻️ Minor cleanup
- const product = productVariant?.product; + const product = productVariant.product;packages/integrations/medusajs/src/modules/products/products.service.ts (3)
52-65: Remove redundant.catchblocks that just re-throw.The
.catch((error) => { throw error; })on lines 58–60 (and similar patterns on lines 82–84, 102–104, 161–163) is a no-op — it catches the error only to re-throw it, which the promise already does by default. The outer RxJScatchErrorin the pipe handles the error propagation tohandleHttpError.♻️ Example cleanup for getProductList
return from( this.sdk.admin.product .list(params) .then((response) => { return mapProducts(response, this.defaultCurrency, query.category); - }) - .catch((error) => { - throw error; }), ).pipe( catchError((error) => { return handleHttpError(error); }), );Apply the same cleanup to
getProductById,getProductByHandle, andgetVariant.
24-27: Field specification strings are fragile — consider documenting or structuring.The
productListFieldsandproductDetailFieldsstrings use Medusa-specific prefixes (*for expansion,+for inclusion) that are easy to get wrong. A brief inline comment explaining the syntax or linking to Medusa docs would help future maintainers.
68-78: Handle vs. ID routing relies onprod_prefix convention.The heuristic
params.id.startsWith('prod_')is reasonable given Medusa's ID convention, but a handle that coincidentally starts withprod_would be misrouted. This is unlikely but worth a brief inline comment noting the assumption.
… and integrate SDK
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
michnowak
had a problem deploying
to
storybook-preview
GitHub Actions
Error
…g and localization
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/integrations/mocked/src/modules/products/products.mapper.ts (1)
273-323:⚠️ Potential issue | 🟡 Minor
mapRelatedProductsdoesn't apply variant overrides, unlikemapProducts.
mapProducts(Lines 225-238) applies first-variant overrides and updates the link for products with variants, butmapRelatedProductsreturns them as-is. If a related product has variants, itslinkwon't include the variant slug andvariantIdwill be absent, which could break the frontend variant routing.
🤖 Fix all issues with AI agents
In `@packages/blocks/product-details/src/frontend/ProductDetails.client.tsx`:
- Around line 25-26: The prop productId is being destructured from the component
props but never used; either remove productId from the destructuring or
explicitly document why it's excluded from ...component (e.g., add a short
inline comment near the destructuring in ProductDetails.client.tsx stating
"intentional exclusion of productId from ...component"), and if the prop is
actually needed instead, wire it into the component logic/JSX where appropriate;
update the destructuring to match the chosen approach so no unused variable
remains.
- Line 35: currentVariantSlug can be undefined if product.variantId doesn't
match any variant; update the logic that computes currentVariantSlug (the const
currentVariantSlug = product.variants?.find((v) => v.id ===
product.variantId)?.slug) to provide a safe fallback (e.g., use
product.variants?.[0]?.slug) or ensure the Select shows a placeholder by
rendering <SelectValue placeholder="Select a variant" /> when currentVariantSlug
is undefined; pick one approach and apply it where the Select uses
currentVariantSlug so the UI always shows a value or a clear placeholder.
In `@packages/integrations/medusajs/src/modules/products/products.mapper.ts`:
- Around line 288-292: The mapped variant is using the variant title for the
product name (name: targetProduct.title) which is inconsistent with
mapProduct/mapProducts; change the mapping in the function that produces the
shown diff so that name uses the product-level title (use product?.title as the
primary value, and fall back to targetProduct.title or an empty string if
needed) to match mapProduct and mapProducts behavior.
In `@packages/integrations/medusajs/src/modules/products/products.service.ts`:
- Around line 163-185: Replace the plain Error thrown in getVariant when
response.variant is missing with a proper 404 error so handleHttpError maps it
to NotFound instead of 500: throw a NotFoundException(`Variant ${variantId} not
found for product ${productId}`) (import NotFoundException from `@nestjs/common`)
in the getVariant map block, ensuring getVariant, mapProduct and handleHttpError
continue to be used as before.
In `@packages/integrations/mocked/src/modules/products/products.mapper.ts`:
- Around line 186-200: The code silently falls back to the first variant when a
provided variantId doesn't match; change the selection logic in the block that
computes selectedVariant (where product.variants, variantId and selectedVariant
are used) so that if variantId is defined but no matching variant is found you
raise/return an explicit not-found error (or a 404-equivalent) instead of using
product.variants[0]; keep the existing fallback to the first variant only when
variantId is undefined, and preserve the subsequent usage of
getVariantOverrides(locale, selectedVariant.slug) and the returned object shape
(variantId, sku, link).
🧹 Nitpick comments (9)
packages/integrations/mocked/src/modules/products/products.mapper.ts (1)
173-178: Locale-to-source selection is duplicated three times.Consider extracting a small helper like
getProductsSource(locale)to keep this DRY and avoid drift if more locales are added.const getProductsSource = (locale?: string) => { if (locale === 'pl') return MOCK_PRODUCTS_PL; if (locale === 'de') return MOCK_PRODUCTS_DE; return MOCK_PRODUCTS_EN; };Also applies to: 208-213, 276-281
packages/blocks/product-details/src/frontend/ProductDetails.client.tsx (2)
30-31:createNavigation(routing)is called on every render — consider memoizing or hoisting.
createNavigationis a factory that returns hooks (useRouter, etc.). Invoking it inside the component body means a new set of hook wrappers is allocated on every render. While functionally correct (React identifies hooks by call order), this is unnecessary work. Ifroutingis stable across renders, consider memoizing the result or lifting the call outside the component.Memoize the navigation factory result
export const ProductDetailsPure: React.FC<ProductDetailsPureProps> = ({ locale, routing, hasPriority, productId, ...component }) => { const { product, labels, actionButton } = component; const t = useTranslations(); - const { useRouter } = createNavigation(routing); - const router = useRouter(); + const navigation = React.useMemo(() => createNavigation(routing), [routing]); + const router = navigation.useRouter();
75-93: Hardcoded'Variant'fallback breaks i18n; also, selector JSX is duplicated.Lines 78 and 196 both use
labels.variantLabel || 'Variant'with a hardcoded English string as fallback. SinceuseTranslations()(t) is already available, prefer a translated fallback liket('product.variantLabel')to keep the component localizable.Additionally, the variant selector block (lines 75–93 and 193–211) is copy-pasted verbatim. Consider extracting it into a small local component or helper to reduce duplication and simplify future changes.
Extract and fix i18n
+ const VariantSelector = () => ( + <div className="flex flex-col gap-2"> + <Typography className="text-sm text-muted-foreground"> + {labels.variantLabel || t('product.variantLabel')} + </Typography> + <Select value={currentVariantSlug} onValueChange={handleVariantChange}> + <SelectTrigger> + <SelectValue /> + </SelectTrigger> + <SelectContent> + {product.variants!.map((variant) => ( + <SelectItem key={variant.id} value={variant.slug}> + {variant.title} + </SelectItem> + ))} + </SelectContent> + </Select> + </div> + );Then in both locations, replace the duplicated block with:
{product.variants && product.variants.length > 1 && <VariantSelector />}packages/integrations/medusajs/src/modules/products/products.mapper.ts (2)
77-113:JSON.parseresult is not validated to be an array before the type annotation takes effect.On line 95,
JSON.parse(rawKeySpecs)can return any valid JSON value (object, number, boolean, etc.), but it is assigned directly tokeySpecstyped as{ value?: string; icon?: string }[]. TheArray.isArrayguard on line 105 catches this at runtime, but between lines 95 and 104 the variable holds an unvalidated value with a misleading type.A minor safety improvement:
Suggested fix
if (typeof rawKeySpecs === 'string') { try { - keySpecs = JSON.parse(rawKeySpecs); + const parsed: unknown = JSON.parse(rawKeySpecs); + if (!Array.isArray(parsed)) { + return undefined; + } + keySpecs = parsed; } catch { return undefined; } } else if (Array.isArray(rawKeySpecs)) {
161-217: Mapping logic is duplicated acrossmapProduct,mapProducts, andmapRelatedProducts.Each mapper independently constructs the same shape (
id,name,image,images,price,link,type,category,tags) with slight variations and inconsistencies (e.g.,shortDescriptiondefaults to''inmapProductbutundefinedinmapRelatedProducts;namesource differs as noted above). Consider extracting a sharedmapBaseProducthelper that both mappers delegate to, reducing divergence risk.Also applies to: 280-325
packages/integrations/medusajs/src/modules/products/products.service.ts (4)
154-161:slugifyis duplicated between the service and the mapper.This method (lines 154-161) is character-for-character identical to the module-level
slugifyinproducts.mapper.ts(lines 11-18). Extract it into a shared utility (e.g.,utils/slugify.ts) and import it in both places to avoid divergence.
58-60: Remove no-op.catchblocks that only re-throw.Each of these
.catch((error) => { throw error; })blocks is a no-op — the error is caught and immediately re-thrown, adding noise without altering behavior. ThecatchErrorin the outerpipe(...)already handles errors.Example for getProductList (apply similarly to others)
return from( this.sdk.admin.product .list(params) .then((response) => { return mapProducts(response, this.defaultCurrency, query.category); - }) - .catch((error) => { - throw error; }), ).pipe(Also applies to: 82-84, 102-104, 171-173
68-78: Handle-vs-ID dispatch relies on a hardcodedprod_prefix convention.Line 70 uses
params.id.startsWith('prod_')to distinguish product IDs from handles. If Medusa's ID prefix convention ever changes, this will silently misroute requests. Consider extracting this as a named constant or utility, and adding a comment referencing the Medusa convention for future maintainers.
24-27: Field strings are fragile — consider documenting or validating them.
productListFieldsandproductDetailFieldsare comma-separated field selector strings specific to the Medusa Admin API. A typo or stale field name after a Medusa SDK upgrade would silently return incomplete data. Consider adding a brief inline comment per field group explaining which model properties each selector targets, to make future SDK upgrades easier to audit.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/blocks/product-details/src/frontend/ProductDetails.client.tsx`:
- Around line 67-68: The component currently calls createNavigation(routing) on
every render (const { useRouter } = createNavigation(routing); const router =
useRouter();), which recreates hook references; hoist the createNavigation call
to module scope by importing the canonical routing config (or a shared
navigation instance) and call createNavigation(...) once outside the component
to get useRouter, then use that stable useRouter inside
ProductDetails.client.tsx; if routing must remain a prop, produce a memoized
navigation object (e.g., memoize createNavigation(routing) by routing identity)
so the returned useRouter reference is stable before calling its hook.
- Line 194: The fallback hardcoded string in ProductDetails.client.tsx uses
labels.variantLabel || 'Variant', which bypasses i18n; replace the literal
fallback with a translation call (e.g. labels.variantLabel ||
t('product.variant')) and ensure the component uses the translation helper (t)
or hook already present in this file so missing labels render localized text;
add the 'product.variant' key to the locale files if it doesn't exist.
- Around line 33-36: The current fallback picks the first variant that matches
only the changed option (via variants.find and withChanged), ignoring other
selections; change resolveVariantSlug to perform a best-effort match instead:
iterate variants and compute a match score by counting how many option keys in
selectedOptions equal variant.options (including the changedOptionId), select
the variant with the highest score (tie-breaker: prefer exact match or keep
current order), and return its slug; if the best score is 0 (no matching
options) return null so the UI can handle an unavailable combination; use the
existing symbols variants, selectedOptions, changedOptionId and return slug or
null accordingly.
- Around line 170-186: The SelectItem list renders unavailable values (computed
via availableValuesPerGroup.get(group.id)?.has(value)) with reduced opacity but
leaves them selectable; update the map loop that renders group.values inside
ProductDetails.client (the block creating <SelectItem key={value} value={value}
...>) to set the SelectItem disabled when !isAvailable so users cannot pick
invalid combinations—use the same isAvailable boolean and add disabled={
!isAvailable } (or the SelectItem equivalent) and keep the opacity class for
visual feedback; this prevents invalid selections that later cause
resolveVariantSlug exact-match failures.
In `@packages/integrations/medusajs/src/modules/products/products.service.ts`:
- Around line 80-100: In getProductById, guard against response.product being
undefined before accessing product.variants: after the SDK retrieve call
resolves in the switchMap, check if response.product exists and if not throw a
clear Error (e.g., "Product not found: <productId>") so the
catchError/handleHttpError path receives a meaningful error; mirror the
existence check used in getProductByHandle and then proceed to validate variants
and call getVariant.
🧹 Nitpick comments (5)
packages/blocks/product-details/src/frontend/ProductDetails.client.tsx (1)
154-211: Variant selector UI is duplicated between mobile and desktop layouts.Lines 154–211 (mobile) and 311–368 (desktop/sticky panel) contain nearly identical variant selection markup. Extract a shared
VariantSelectorcomponent to keep the two layouts in sync and reduce maintenance burden.packages/integrations/medusajs/src/modules/products/products.service.ts (3)
52-65: Remove no-op.catchblocks on SDK promises.The
.catch((error) => { throw error; })pattern appears on every SDK call (lines 58–60, 84–86, 106–108, 162–164). Re-throwing the same error is a no-op — the rejection already propagates throughfrom()into the RxJScatchErrorin the pipe. This adds noise without changing behavior.♻️ Example fix for getProductList
return from( this.sdk.admin.product .list(params) .then((response) => { return mapProducts(response, this.defaultCurrency, query.category); - }) - .catch((error) => { - throw error; }), ).pipe(
145-152: Duplicateslugifyimplementation — consider sharing.This
slugifymethod is character-for-character identical to the module-levelslugifyfunction inproducts.mapper.ts(lines 11–18). Extract it into a shared utility to avoid drift.
68-78:params.variantIdis reinterpreted as a slug when routing by handle — consider documenting this dual meaning.When
iddoesn't start withprod_,params.variantIdis passed togetProductByHandleasvariantSlug. This implicit reinterpretation works for the URL scheme (/products/{handle}/{variantSlug}) but is easy to misunderstand since the field is namedvariantId. A brief inline comment clarifying this would help future maintainers.packages/integrations/medusajs/src/modules/products/products.mapper.ts (1)
63-99:mapKeySpecsFromMetadatalacks type validation on parsed array items.Line 81 parses a JSON string into
keySpecsand line 86 assignsrawKeySpecsdirectly — both assume array items have the shape{ value?: string; icon?: string }. Malformed metadata (e.g., items that are numbers or strings instead of objects) would produce silently incorrectKeySpecItementries withundefinedfields rather than being filtered out.Consider adding a guard on the mapped items:
🛡️ Proposed improvement
return keySpecs + .filter((spec): spec is Record<string, unknown> => spec != null && typeof spec === 'object') .map((spec) => ({ - value: spec.value, - icon: spec.icon, + value: typeof spec.value === 'string' ? spec.value : undefined, + icon: typeof spec.icon === 'string' ? spec.icon : undefined, }));
# Conflicts: # package-lock.json
3 participants
What does this PR do?
Related Ticket(s)
Key Changes
How to test
Media (Loom or gif)
Summary by CodeRabbit
New Features
Updates