Conversation
🦋 Changeset detectedLatest commit: 9987270 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Unlighthouse Performance Comparison — VercelComparing PR preview deployment Unlighthouse scores vs production Unlighthouse scores. Summary ScoreAggregate score across all categories as reported by Unlighthouse.
Category Scores
Core Web Vitals
|
Bundle Size ReportComparing against baseline from No bundle size changes detected. |
|
Question: since |
|
|
||
| const localeNodes = buildConfig.get('locales'); | ||
|
|
||
| function pathFromFullPath(fullPath: string | null | undefined): string | undefined { |
There was a problem hiding this comment.
We can output also path if needed, this is an easy task.
store {
locales{
code : 'fr'
isDefault : true
path: 'france'
fullPath : 'site.com/france'
}
}
There was a problem hiding this comment.
That would be great, it would be cleaner to have the path output as well
| return parsed.data.graphqlErrors.some((e) => e.message.includes("Cannot query field 'fullPath'")); | ||
| } | ||
|
|
||
| async function fetchSettings() { |
There was a problem hiding this comment.
Is this the standard pattern for new GQL schemas in storefront? I would double check if we can just change the schema without the flag, it should just return null anyway. May not be possible.
There was a problem hiding this comment.
I didn't love this change either because we'll have to go back and clean it up once the flag graduates... BC's storefront removes from the schema entirely when the flag is off. I agree it feels weird and I feel as though this issue probably comes up often i'll send a message in trac-team to see if anyone else handled this differently.
There was a problem hiding this comment.
Hey Parth, looks like this is standard pattern on the storefront side (see SearchSchema.scala:28). Thanks for the review.
| import { defaultLocale, locales } from './locales'; | ||
|
|
||
| enum LocalePrefixes { | ||
| ALWAYS = 'always', |
There was a problem hiding this comment.
Just curious: why did we remove this?
There was a problem hiding this comment.
I didn't think we needed the enum anymore as we are only using as-needed. I can add a comment like this if you prefer:
localePrefix: {
// 'as-needed' removes the prefix for the default locale.
// 'always' would prefix every locale including the default.
// 'never' has cache-related issues — avoid:
// amannn/next-intl#786
mode: 'as-needed',
prefixes,
}
There was a problem hiding this comment.
Enum change reverted to provide context for merchant configuration
What / Why
PROJECT-5002 added merchant-configurable per-locale URL subfolders in the BC Control Panel — e.g.,
fr-CAcan be served at/fr/instead of/fr-CA/. The Storefront GraphQL API exposes this asLocale.fullPath.This PR teaches Catalyst to use that configured subfolder for URL routing, while keeping the locale code for translations,
<html lang>,Accept-Language, and BC content negotiation.JIRA
TRAC-421
Testing
Tested 5 cases in Integration.
Case 1 — No custom subfolders (legacy)
{locale = fr, isDefault=true, path=""}
{locale = es-es, isDefault=false, path=""}
{locale = es-mx, isDefault=false, path=""}
{locale = uk, isDefault=false, path=""}
Expected URLs:
site.com/ <-- fr locale
site.com/es-es/ <-- es-es locale
site.com/es-mx/ <-- es-mx locale
site.com/uk/ <-- uk locale
Tested: [X]
Case 2 — One non-default has a subfolder
{locale = fr, isDefault=true, path="" }
{locale = es-es, isDefault=false, path="es"}
{locale = es-mx, isDefault=false, path=""}
{locale = uk, isDefault=false, path=""}
Expected URLs:
site.com/ <-- fr locale
site.com/es/ <-- es-es locale
site.com/es-mx/ <-- es-mx locale
site.com/uk/ <-- uk locale
Tested: [X]
Case 3 — Default and one non-default have subfolders
{locale = fr, isDefault=true, path="fr" }
{locale = es-es, isDefault=false, path="es"}
{locale = es-mx, isDefault=false, path=""}
{locale = uk, isDefault=false, path=""}
Expected URLs:
site.com/fr/ <-- fr locale
site.com/es/ <-- es-es locale
site.com/es-mx/ <-- es-mx locale
site.com/uk/ <-- uk locale
Note: nobody lives at bare /. Hitting site.com/ redirects to site.com/fr/.
Tested: [X]
Case 4 — Every non-default has a subfolder, default blank
{locale = fr, isDefault=true, path="" }
{locale = es-es, isDefault=false, path="es"}
{locale = es-mx, isDefault=false, path="mx"}
{locale = uk, isDefault=false, path="uk"}
Expected URLs:
site.com/ <-- fr locale
site.com/es/ <-- es-es locale
site.com/mx/ <-- es-mx locale
site.com/uk/ <-- uk locale
Tested: [X]
Case 5 — Default has a path, one non-default left blank
{locale = fr, isDefault=true, path="fr" }
{locale = es-es, isDefault=false, path="es"}
{locale = es-mx, isDefault=false, path="ex-mx"}
{locale = uk, isDefault=false, path=""}
Expected URLs:
site.com/fr/ <-- fr locale
site.com/es/ <-- es-mx locale
site.com/es-mx/ <-- es-mx locale
site.com/ <-- uk locale
Tested: [X]
Migration
None needed.