sync integrations/makeswift with canary - #3198
Merged
Merged
Conversation
…ct rules (#3131) Merging canary into integrations/makeswift silently overwrites core/package.json's nested catalyst.version/catalyst.ref fields with canary's raw value instead of the makeswift package's own identity, since this hunk doesn't produce a textual conflict for git to flag. Document the field as something to check by hand on every sync. Refs TRAC-1280 Co-authored-by: Claude <noreply@anthropic.com>
Replace the branch-specific /sync-makeswift skill (and the proposed /sync-b2b-makeswift skill) with a single parameterized /sync-integration-branch skill. The target integration branch is passed as an argument; SKILL.md holds the generic sync flow, and each supported branch has a reference file (references/<name>.md) with its upstream, package identity, integration surface, and branch-specific caveats. Phase 0 resolves the target and loads the matching reference. When no branch is given, or the branch is unknown or absent on origin, the skill lists the valid integrations/* targets and asks the user to pick one. Point the release-catalyst skill at the new skill name. Refs TRAC-1236 Co-authored-by: Claude <noreply@anthropic.com>
…yst (#3133) The release-catalyst skill released catalyst-core and catalyst-makeswift but had no stage for catalyst-b2b-makeswift, so nothing moved its @latest tag (which the CLI upgrade.ts resolves). Add a b2b sync-and-release stage mirroring the makeswift one (sourced from integrations/makeswift), and extend the @latest tag-push and cleanup stages to cover the b2b package. Note the large-catch-up cadence: one b2b release per makeswift minor tag. Refs TRAC-1290 Co-authored-by: Claude <noreply@anthropic.com>
* feat(other): LOCAL-1444 delivery translation * chore(core): create translations patch --------- Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
…dant auth login (#3164)
* TRAC-1368: fix(client) - Surface clear error for non-JWT storefront token A 401 from the storefront GraphQL API previously threw a bare BigCommerceAPIError with no indication of the cause. When BIGCOMMERCE_STOREFRONT_TOKEN is not a storefront JWT (e.g. an OAuth access token was supplied instead), this left developers to reverse-engineer that the token type mattered. Detect when a 401 response coincides with a malformed storefront token and throw InvalidStorefrontTokenError, which explains that a storefront JWT is required and links to how to generate one. Refs TRAC-1368 Co-Authored-By: Claude <noreply@anthropic.com> * TRAC-1368: ref(client) - Stop overclaiming JWT-shape check validates token isWellFormedStorefrontToken only verified JWT shape (three base64url segments, JSON-decodable payload) but the name and comment implied it confirmed the token was a valid storefront token. The storefront service accepts several distinct JWT-based token kinds (Simple, Private, Customer Impersonation, etc, per bigcommerce/storefront's GraphqlToken.scala), so shape alone can't prove storefront-token validity — only that the token is a JWT at all, which is what the original reported bug (an opaque OAuth token) actually violates. Rename to looksLikeJwt, rewrite the doc comment to state this limitation explicitly, and soften the InvalidStorefrontTokenError message accordingly so it doesn't overclaim either. No behavior change. Refs TRAC-1368 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
…3155) The baseline-branch detection step only special-cased @bigcommerce/catalyst-makeswift; every other package name, including @bigcommerce/catalyst-b2b-makeswift, fell through to canary. Since integrations/b2b-makeswift carries substantial B2B-only code on top of makeswift, comparing its bundle against canary produced a misleading diff on every b2b-makeswift PR. Add an elif branch mapping @bigcommerce/catalyst-b2b-makeswift to integrations/b2b-makeswift, mirroring the existing pattern in changesets-release.yml's package-resolution step. Refs LTRAC-1406 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* TRAC-1264: fix(cli) - Show progress feedback while creating a channel The channel-creation API call provisions the channel and, when requested, seeds sample data — both can take several seconds. With no spinner, the CLI appeared frozen after the last prompt. Add a consola.start/success pair around the call, matching the existing "Fetching channels..." pattern used elsewhere in the CLI. Fixes TRAC-1264 Co-Authored-By: Claude <noreply@anthropic.com> * TRAC-1264: style - Fix import order lint warning in create-channel-flow.spec.ts Refs TRAC-1264 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
… directives (#3171) * fix(core): TRAC-1425 Don't strip Expires from session cookie deletion directives stripSessionCookieExpiry was stripping Expires from all session token Set-Cookie headers, including deletion directives (empty value + Expires=past). This turned cookie deletions into permanent empty-value session cookies, so the browser never removed them. Stale cookies then persisted through logout, causing session confusion and cart state issues. Narrow the regex with [^;] after = so headers where the value is empty (deletion directive) are not matched, preserving their Expires=past or max-age=0 so the browser actually removes the cookie. Fixes TRAC-1425 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: TRAC-1425 Add changeset Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
… setup (#3178) setupCommerceHosting built a fresh object and wrote it over .bigcommerce/project.json, so every key it doesn't set itself was dropped: the `env` block managed by `catalyst env`, the persisted `apiHost`, and stored credentials whenever setup ran without them. The costly one is `env`. Those entries are sent as secrets on every `catalyst deploy`, so losing them doesn't just lose local config -- the deploy that wiped them proceeds, and the next one ships a worker with no BIGCOMMERCE_STOREFRONT_TOKEN or BIGCOMMERCE_CHANNEL_ID. Nothing in the output mentions env vars, so the first sign is a broken storefront. It fires during ordinary use. deploy.ts calls setup whenever getProjectState().isTransformed is false, behind a prompt defaulting to yes, and isTransformed needs all three of middleware.ts present, proxy.ts absent, and @opennextjs/cloudflare installed. Any one of those flipping -- a partly reverted project, a merge restoring proxy.ts, an interrupted setup -- is enough. `projects link` reaches the same path. Now merges into whatever is already there. Unknown keys are carried through untouched so a later addition to the config doesn't have to remember this call site, and absent storeHash/accessToken means "not supplied on this run" rather than "clear it". A corrupt or non-object file falls back to starting fresh, since setup writes everything a working project needs. Not switched to getProjectConfig(), which would merge for free via Conf: its schema enforces `format: 'uuid'` on projectUuid, which would turn a malformed-uuid case into a hard failure and change behaviour well beyond this fix. Worth considering separately. create.ts already worked around this by ordering its Conf writes after setup; that comment described the old overwrite semantics, so it is updated rather than the ordering changed. Verified the three preservation specs fail against the previous behaviour. Refs LTRAC-1582 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(other): LOCAL-1444 delivery translation * chore(core): create translations patch --------- Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
…ntries (#3176) * LTRAC-1583: build(core) - Add a unit test harness to core core has never had one. `tests/` holds 63 Playwright specs run by `playwright test`; there was no `test` script, no runner, and no way to unit test anything under `lib/`. Scoped to `lib/**/*.spec.ts` deliberately. Playwright specs share the `*.spec.ts` suffix, so an unscoped Vitest would collect all 63 and fail on their fixture imports. This glob is the part worth reviewing closely: widen it and Vitest swallows the e2e suite, narrow it and specs silently stop running. Colocated rather than under `tests/`, matching how packages/catalyst already does unit tests -- build.spec.ts next to build.ts, and so on. `tests/` is Playwright's testDir, so specs placed there would need either a separate suffix or a testIgnore entry to keep the two runners apart; and `tests/lib/` already means "helpers for Playwright tests", which would make `tests/lib/kv` ambiguous. The `~` alias mirrors the tsconfig path alias so lib code importing `~/...` resolves the same way it does in the app. Kept as its own commit so the harness is separable from the fix that follows, but shipped in the same PR: a runner with no specs passes CI vacuously and proves nothing, which is a poor thing to ask a reviewer to approve. Refs LTRAC-1583 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * LTRAC-1583: fix(core) - Expire in-memory KV entries so processes share refreshes MemoryKvAdapter doubles as an L1 in front of whichever shared adapter is selected, and KV.mget skips the shared store whenever memory holds every requested key. Nothing ever expired those entries, so that short-circuit was permanent. That left the `expiryTime` callers embed in the value as the only thing driving refreshes -- and that path refetches from the origin, not from the shared store. So each process ran its own refresh loop on a clock starting from whenever it first cached the key, and never picked up a value another process had already fetched and written. Observed on a deployed store: storeStatus was written at 01:19:01 with a window ending 01:24:01, and at 01:21:12 a request read the *previous* value and refetched again. That read never reached the shared store despite a fresher value having been there for two minutes. A later trace showed a route entry served from memory 14 hours stale. Nothing was ever served incorrectly -- staleness stayed bounded by the caller's own expiryTime, and stale-while-revalidate still served fast and refreshed behind the request. The cost was duplicated work: origin requests and cache writes scaling with process count instead of being shared. 60s matches Workers KV's floor for `cacheTtl` on a read, so the two layers share one staleness window rather than one each. It sits inside the shortest window callers embed in their values (5 minutes for storefront status, 30 for routes). The trade is one shared-store read per key per window against a value nothing ever replaces; those reads become near-free once CloudflareKvAdapter passes `cacheTtl`, which serves them from the colo cache. Capacity goes from 500 to 4096. Cache keys include the query string, so distinct keys accumulate much faster than the number of real paths suggests -- a crawler walking `?utm_*` permutations alone can churn through the old limit. Not using allowStale/noDeleteOnStaleGet, which would serve an expired entry instantly and refresh it off the response path: that needs a `waitUntil` handle to run the refresh on, and KV.mget is called from with-routes without the request's event. Returning stale values with no refresh would reinstate the bug this fixes. Expiry now comes from lru-cache rather than being tracked by hand. The previous expiresAt bookkeeping was only consulted by a private `get` that nothing called -- KV.get delegates to mget -- so it expired nothing, and a caller passing `ex` would have had it silently ignored. `ex` now maps to a per-entry TTL override. mget also returns null rather than undefined for a miss, matching KvAdapter. Two things worth knowing about the tests. lru-cache captures a reference to the `performance` object at module load, and Vitest's fake timers swap the global for a new object that reference never sees -- so the cache's clock has to be moved by stubbing the method in place. And lru-cache treats a recorded start time of exactly 0 as "no TTL", so the fake clock starts at a non-zero baseline; starting at zero makes every expiry assertion pass whether or not the TTL works. Verified both the TTL and the capacity change fail their specs when reverted. Refs LTRAC-1583 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ntries (#3180) The 60s window added in LTRAC-1583 was meant for the L1 copy in front of a shared store, so a process periodically re-reads it and picks up values other processes wrote. But createKVAdapter falls back to the same adapter class when no shared store is configured, so both layers got the window and now empty together. That matters because with-routes treats a missing entry differently from a stale one: a stale entry refreshes in the background via waitUntil, a missing one blocks on `await updateRouteCache(...)`. With both layers empty the value is gone rather than stale, so every request past the window takes the blocking branch -- a 30 minute background refresh became a blocking Storefront API call every 60 seconds, roughly 30x the origin requests and now on the request path. Reproduced against canary: kv.set() then kv.get() 61s later returns null, where before LTRAC-1583 it returned the value. Hits any deployment with no CATALYST_ROUTES_KV binding, no Upstash, and not on Vercel: self-hosted without Upstash, native hosting until the binding ships, and local dev. The window is now opt-in. It exists so the L1 defers to a shared store; where the fallback is itself in-process memory there is nothing to defer to, so it stays unbounded as it was. MemoryKvAdapter takes { ttlMs }, KV's L1 passes SHARED_STORE_RECHECK_MS, the fallback passes nothing. No changeset: LTRAC-1583 is merged but unreleased, so its changeset is still pending on canary. This corrects that work before it ships rather than changing anything a consumer has seen, and a second entry would describe a fix for a bug that never reached a release. The regression test exercises the real `kv` singleton rather than the adapter alone, because the bug is in how the two layers compose -- every spec in LTRAC-1583 constructed the adapter directly, which is exactly why nothing caught this. Verified it fails when the fallback is given the window back. Refs LTRAC-1617 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* TRAC-1298: Update locale paths to runtime resolution * TRAC-1298: Clean up comments
* feat(other): LOCAL-1444 delivery translation * chore(core): create translations patch --------- Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
… immutably (#3184) `templates/public_headers` has always specified the right policy for /_next/static/*, but nothing copied it into the build output. build.ts wrote only open-next.config.ts and wrangler.jsonc, and no _headers file existed anywhere in the repo. Without it Workers Assets applies its own default. Measured on a deployed store, all 32 hashed assets on a product page returned: cache-control: public, max-age=0, must-revalidate That is wrong by construction for a content-hashed filename — the hash is the version, so a URL can never return different bytes. It forced a conditional request for all 32 assets on every repeat page view (~75ms each, all 304 Not Modified), including 4 render-blocking CSS files and 3 fonts. The intended public,max-age=31536000,immutable removes them. The header is missing in the first place because /_next/static/* never reaches the Next.js server on Workers; Cloudflare's asset layer serves it directly, so Next's own immutable header never applies. _headers is the supported override, and OpenNext's own migrate command generates a byte-identical public/_headers, treating it as the app's job. Copy the template to .open-next/assets/_headers after the OpenNext build, since that build regenerates the directory, and before the Wrangler dry-run so Wrangler validates it. The existing recursive copy into .bigcommerce/dist/assets carries it into the uploaded bundle. Also add a regression test asserting the copy happens. The bug was an absent value rather than a wrong one, with nothing asserting it — the new test was verified to fail when the copy is removed. Refs LTRAC-1462 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ts (#3183) `cachePurge` was declared in the generated open-next.config.ts but never had credentials on native hosting, so every purge no-opped with "No cache zone ID or API token provided. Skipping cache purge." — confirmed in the production Worker logs of a deployed store. Correctly invalidating a regional (Cache API) entry needs one of two mechanisms: a CDN purge that evicts it, or a tag-cache check on every hit. doShardedTagCache.writeTags() always writes to its DO shards and always clears the regional tag cache; only the CDN purge is gated behind isPurgeCacheEnabled(). So purge exists specifically to evict incremental cache entries — exactly the check bypassTagCacheOnCacheHit was skipping. Catalyst was configured for purge and had neither. Declaring cachePurge without credentials was worse than omitting it: isPurgeCacheEnabled() only checks whether it is declared, so OpenNext believed purge was handling invalidation and disabled shouldLazilyUpdateOnCacheHit (documented as on by default for 'long-lived'). A hit was then neither purged, nor refreshed from R2, nor tag-checked, so revalidateTag had no effect on it until max-age expired. Remove both to take the tag-checking route: the tag cache is consulted on a hit and the entry refreshes from R2 in the background. Costs an extra tag-cache and R2 read per hit, which is what those options were trading for correctness. Purge is still the faster mechanism and worth restoring later, but not by supplying credentials: it needs a Cloudflare API token bound into the Worker, a binding is readable by the merchant's own application code, and purge is zone-scoped across every tenant on the shared hosting zone. That needs a design keeping the credential out of tenant Workers. The NEXT_CACHE_DO_PURGE binding is intentionally left in place: OpenNext's worker template exports all three DO classes unconditionally, so it still resolves and needs no Durable Object migration. It is inert until purge returns. Refs LTRAC-1458 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t via entry_type (#3190)
#3192) Every Catalyst tag ships core/package.json with `workspace:` specifiers, because @bigcommerce/catalyst-core is private and pnpm's publish-time rewrite never runs on the tagged tree. Both sides of the 3-way merge therefore agreed on those lines, so a project's @bigcommerce/catalyst* dependency versions never moved through an upgrade: flat projects kept whatever `catalyst create` resolved at scaffold time, and monorepo- structure projects kept `workspace:^` indefinitely. Read the versions each tag actually published from that tag's own tarball and pin the downloaded base and target trees to them, so the existing merge carries the bump like any other change, or conflicts when the merchant pinned deliberately. A dependency the project still holds as `workspace:` is skipped on both sides, since normalizing one side alone would manufacture a conflict on something that works fine as-is; those are surfaced instead as an opt-in migration prompt. Also add two advisories the merge cannot cover: a reminder to reinstall when the upgrade changed package.json, and a notice when the pinned @bigcommerce/catalyst is behind the published version, which it always will be eventually since `catalyst create` pins it exactly and the dependency does not exist upstream for a merge to touch. Refs TRAC-1536 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ue (#3193) * LTRAC-1457: fix(catalyst) - Revalidate via self-fetch instead of a DO queue OpenNext's doQueue routes ISR revalidation through a Durable Object whose constructor reads env.WORKER_SELF_REFERENCE and throws without it. queueCache.send() catches that error and only logs, so revalidation would fail silently rather than surface. That binding cannot point at this Worker on native hosting. A Cloudflare service binding resolves against account-level Workers, and a Catalyst deployment is a script inside a dispatch namespace, which is not addressable that way. Adding it was attempted and rejected at upload: 400 Bad Request code 10143 Service binding 'WORKER_SELF_REFERENCE' references Worker '…' which was not found. Two alternatives were considered and rejected on security grounds, both because they would expose other tenants: - A dispatch_namespace binding. Besides the API mismatch — OpenNext calls .fetch() while that binding exposes .get(name) — .get() accepts any script in the namespace, letting any deployment invoke any other. - A service binding to the dispatch router, which is account-level and would resolve, routing back here by hostname and keeping the Durable Object queue intact. It would also hand every tenant a handle able to reach any other tenant's Worker. Public fetches already reach those endpoints, but only through the Cloudflare edge; an internal handle bypasses WAF and rate limiting, so it is not equivalent. Revalidation does not require a binding. It is a HEAD request to the page's own public URL carrying the build-time preview secret, which is exactly what the Durable Object issues once it holds the service handle. Issue it with a plain fetch instead: the subrequest leaves and re-enters through the dispatch router and arrives at the same Worker, and global_fetch_strictly_public is already set so it is not short-circuited internally. Still wrapped in queueCache, so concurrent stale hits for one path collapse into a single revalidation. Bounded by a 10s timeout matching the Durable Object's default, and a non-ok response throws so a failed regeneration is logged and retried on the next stale hit rather than silently cached as done. Lost relative to the Durable Object: retry, max-concurrency, cross-region dedup, and failed-route backoff. Accepted because the alternative is not "keep the Durable Object" — it is no revalidation at all. NEXT_CACHE_DO_QUEUE is left bound. OpenNext's worker template exports all three DO classes unconditionally, so it still resolves and needs no migration; it is simply inert. Latent until ISR is adopted — the build currently produces no routes with a revalidate window, so the queue is never invoked. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LTRAC-1457: fix(catalyst) - Silence no-underscore-dangle on Next's env var `__NEXT_PREVIEW_MODE_ID` is inlined by Next at build time, so the leading underscores are its naming, not ours, and renaming is not an option. Scoped to the single line rather than the file so the rule keeps applying to the rest of the template. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LTRAC-1457: docs(catalyst) - Lead the changeset with the no-behavior-change note The caveat was the last line of a long entry, so a skim read like a caching improvement — easy to confuse with the regional-cache tag-checking change, which cost latency rather than saving it. State up front that no route has a revalidate window today, cite the prerender manifest, and distinguish fetch-level `next: { revalidate }` from route-level ISR. Frame the fix as removing a silent-failure trap, since the `IgnorableError` is dropped under the default log threshold. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LTRAC-1457: docs(catalyst) - Trim rejected alternatives from the config comment The header carried the full case against the service binding and the dispatch router, including the 10143 upload error and the cross-tenant reach argument. That is review context, not something a reader of this file needs, and it is already stated at length in the pull request. Keep only what the code cannot convey on its own: that `doQueue` is unusable here, the mechanism the queue replaces it with, and that `global_fetch_strictly_public` is load-bearing — without it the subrequest would be short-circuited and a future reader might reasonably simplify the plain `fetch` away. Point at LTRAC-1457 for the rest. Also separate the queue comment from the `REVALIDATION_TIMEOUT_MS` one, which ran together and read as a single block attached to the constant. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * LTRAC-1458: docs(catalyst) - Trim the purge rationale from the config comment Same treatment as the queue comment above it, applied to the block that came in with LTRAC-1458. The case against enabling purge — the zone-scoped token being readable by merchant application code on a shared tenant zone — is review context, and PR #3183 states it more completely than this comment did, naming the env vars and linking the ignition attempt that was closed for it. Kept the footgun, which is the part a reader of this file needs: that the omission is deliberate, and that declaring `cachePurge` flips OpenNext's defaults on declaration alone regardless of whether it can authenticate. Without that, restoring it looks like a free speed win and silently reintroduces the bug LTRAC-1458 fixed. Dropped the per-hit read cost, which is quantified in the benchmark report on LTRAC-1468 rather than estimated in a comment. Refs LTRAC-1457 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…artifacts (#3195) `npm run generate` writes `bigcommerce.graphql` and `bigcommerce-graphql.d.ts` to the project root, and `catalyst create` makes its initial commit before anything runs `generate`. Both files therefore appear as untracked the first time a merchant runs `pnpm run dev`, with nothing in the project saying what they are, whether they are read at runtime, or whether the intent was to commit or ignore them. Document in the scaffolded project's README that both files should be committed, name `generate` as the command that produces them, and describe local versus build-time regeneration. Record the same decision in `core/.gitignore`, where their absence otherwise looks like an oversight. Explain in CONTRIBUTING.md why the monorepo does the opposite: contributors work against unreleased Storefront API schemas on their own test stores, so a committed schema here would generate diffs belonging to no change and conflict between `canary` and the `integrations/*` branches. CI regenerates from store credentials before it lints and typechecks. Refs LTRAC-1343 Co-authored-by: Claude <noreply@anthropic.com>
…3196) Tidy the queued changesets so the generated changelog reflects what actually shipped. Bump `ltrac-1298-runtime-locale-subfolders` from patch to minor: it drops locales from `build-config.json` entirely and moves resolution to runtime, which is a behavior change rather than a fix. Drop `ltrac-1343-document-graphql-artifacts`, which covers a README and `.gitignore` comment only and does not belong in a released changelog. Collapse the three identical "Update translations." changesets into one, since three repeated entries say nothing the first does not. None of this moves a version number — catalyst-core is already going minor via the wallet buttons changeset. Refs LTRAC-1700 Co-authored-by: Claude <noreply@anthropic.com>
…3197) The accumulating syncs run unversioned, so complete had no version to target; per Linear support, a pipeline with no natural labeling moment needs one final versioned sync right before complete. Access keys are also per pipeline, so each branch's package now maps to its own secret and unset keys skip the Linear steps instead of writing into the catalyst-core pipeline. Refs TRAC-1601 Co-authored-by: Claude <noreply@anthropic.com>
…akeswift # Conflicts: # core/CHANGELOG.md # core/app/[locale]/layout.tsx # core/i18n/routing.ts # core/package.json # core/proxies/with-intl.ts
🦋 Changeset detectedLatest commit: 2792fcd 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.
|
Contributor
Bundle Size ReportComparing against baseline from
Per-Route First Load JS
|
Contributor
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
|
parthshahp
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings
integrations/makeswiftup to date withcanarythrough the@bigcommerce/catalyst-core@1.11.0release, and adds the@bigcommerce/catalyst-makeswiftminor changeset for it.What/Why?
The bulk of this is LTRAC-1298, which moved locale routing from a build-time snapshot (
i18n/locales.ts,build-config.json) to a per-request lookup (i18n/locale-config.ts,i18n/locale-routing.ts). That deletedi18n/locales.tsand theroutingexport, both of which the Makeswift surface used, so most of the work here is porting Makeswift code onto the runtime API.Conflict resolutions
core/package.json— kept the branch's identity:namestays@bigcommerce/catalyst-makeswift,versionstays1.10.0, and nestedcatalyst.version/catalyst.refstay pinned to@bigcommerce/catalyst-makeswift@1.10.0rather than taking canary's@bigcommerce/catalyst-core@1.11.0.core/CHANGELOG.md— kept the branch's history; latest entry is still## 1.10.0.core/i18n/routing.ts— took canary's version wholesale (it's now just re-exports). The MakeswiftlocaleCookieconfig (partitioned,secure,sameSite: 'none', needed for the Builder canvas) moved intocreateRoutingincore/i18n/locale-routing.ts, which is where both the proxy middleware and client navigation now build their config.core/proxies/with-intl.ts— took canary's per-request routing resolution, and re-applied the Makeswiftx-bc-disable-locale-detectionheader handling by spreadinglocaleDetection: falseovercreateRouting(localeRouting).core/app/[locale]/layout.tsx— canary's structure (getLocaleRouting,LocaleRoutingProvider) with the Makeswift surface preserved:MakeswiftProviderstill wraps<html>,SiteThemestill renders in<head>.pnpm-lock.yaml— took canary's and regenerated.Makeswift code ported off the deleted
i18n/locales.tscore/lib/makeswift/client.ts—normalizeLocaleis nowasyncand readsdefaultLocalefromgetLocaleRouting().core/vibes/soul/primitives/navigation/_actions/localized-pathname.ts— same, resolved inside the existing async functions.core/app/api/products/{[entityId],group/[group],ids}/route.ts— the three Makeswift product API routes resolvedrouting.locales/routing.defaultLocaleat module scope; they nowawait getLocaleRouting()per request.Needs a second opinion
I removed
generateStaticParamsfromcore/app/[locale]/(default)/page.tsxandcore/app/[locale]/(default)/[...rest]/page.tsx. Both fanned Makeswift page paths out over the build-timelocalesarray, which no longer exists. Resolving the list at build time isn't available either —getLocaleRouting()readsheaders(), which isn't callable fromgenerateStaticParams.I took the reading that matches what canary did to
app/[locale]/layout.tsxfor the same reason: every route under[locale]already renders on demand because the tree reads cookies, so the fan-out was adding a build-time dependency on the locale list without actually prerendering anything. If that reasoning doesn't hold for the Makeswift catch-all specifically, this is the hunk to push back on.Testing
pnpm run lintandtsc --noEmitboth pass clean incore/. Prettier is clean on every touched file. I have not run a build or exercised the Builder canvas — the locale-cookie and locale-detection paths above are the ones worth a manual check in Makeswift.Refs LTRAC-1700