120 task performance optimization audit for core page templates#162
120 task performance optimization audit for core page templates#162Arnimag wants to merge 50 commits into
Conversation
Removes data-state fade/zoom Tailwind animation classes and switches the scroll-hint update hook from onAnimationEnd to onOpenAutoFocus (which fires immediately on open, not after an animation that no longer exists).
- tsconfig target bumped to ES2022 for smaller transpilation output
- browserslist added for modern evergreen browsers + Safari 16
- react-query-devtools moved to devDependencies and loaded via next/dynamic
gated on
NODE_ENV === 'development' so it is tree-shaken from production bundles
…tems Adds preload + fetchpriority=high hints on the first product gallery image, logo, and lazy-video poster to improve LCP. PLP product grid accepts eagerCount so the first 4 cards render images eagerly.
… from
cached fetches
Cookie-based draft detection forced every BFF fetch to read cookies(), which
opts every
route out of ISR. Preview now lives behind /preview/[...path] gated by a
token cookie issued only by /api/draft; live routes never read draft state and
are free
to be ISR-cached.
- /api/draft generates a short-lived preview token and redirects to
/<locale>/preview/<slug>
- proxy.ts rewrites live paths → /preview/ while the token cookie is valid
- bff-fetch, bff-fetch-server, and bff-cache-options accept an explicit
isDraft option
instead of inspecting cookies; cookies() is never called on cacheable routes
- feature getters (content, product, productCollection) thread isDraft through
to the BFF
- New parse-search-params.ts extracts PLP query-string parsing (used by the
preview route)
… hoist
html/body
With cookie reads removed from the BFF fetch path (prev commit), locale routes
can now be
ISR-cached. setRequestLocale(locale) is required in every segment because
under ISR a
parent layout may be served from cache without re-executing, so children
cannot rely on
the layout's call.
DM_Sans font,
QueryProvider, and preconnect hints for product + content image CDNs
- app/layout.tsx: collapses to a pass-through (html/body now owned by locale
layout)
- manifest.json moved to /public so it is served as a static asset
- eslint.config.mjs: local require-set-request-locale rule enforces the
contract across
all app/[locale]/**/page.tsx and layout.tsx files
- PRODUCT_IMAGE_HOSTS constant introduced for use in preconnect + next.config
remotePatterns
- Routes that genuinely need request cookies (account/*, cart, checkout/*,
wishlist) get
export const dynamic = 'force-dynamic'; CMS and product pages become
ISR-cacheable
… segment
The data-source demo selector was silently ignored on ISR-cached routes —
every user received the default data-source regardless of their cookie.
Root cause: Next.js ISR keys the page cache by URL + dynamic segments only.
Both variants shared one cache cell and always rendered the default.
Fix: move data-source into the URL as an internal segment injected by
middleware. External URLs stay clean (/en/foo); NextResponse.rewrite()
maps them internally to /commercetools-set/en/foo etc. Each variant gets
its own ISR page-cache cell and fetch-cache entries.
Routing (proxy.ts, app/ tree):
- Middleware resolves data-source from cookie, injects [dataSource] segment
on every rewrite, and strips leaked internal URLs (302).
- Adds Vary: Cookie on all responses so a CDN cannot serve one user's
variant to another.
- Forwards x-correlation-id request header for dynamic routes.
- app/[locale]/** moved to app/[dataSource]/[locale]/**; locale layout reads
dataSource from params and calls setRequestDataSource().
Request context (lib/request-context/data-source.ts):
- React cache()-based store, same pattern as next-intl's setRequestLocale.
Threads the resolved value into BFF calls without reading cookies, which
would opt ISR routes into dynamic rendering.
BFF layer:
- bff-fetch.ts: explicit dataSource option bypasses cookie parsing.
- bff-fetch-server.ts: reads from request context; drops getCorrelationId()
which called headers()/cookies() and silently disabled ISR on every route.
- bff-fetch-client.ts: removed dead correlationId cookie write (no longer
read server-side); client-side tab-scoped ID via sessionStorage unchanged.
- getStoreConfig: wrapped in React.cache() for per-request deduplication,
consistent with getHeaderLayout.
Correlation ID design: user-scoped tracing lives exclusively in
useBffFetchClient (client side). Server BFF calls are either ISR-cached
(not per-request) or shared config fetches; the BFF generates its own ID.
…tions threading
Preview system (cross-site iframe safe):
- /api/draft issues a signed short-lived token; on HTTPS delivered via
HttpOnly SameSite=None cookie (token never in URL), on HTTP via ?__pt=
URL param (only viable without HTTPS for Contentful's cross-site iframe)
- proxy.ts detects /preview/… paths by shape, injects cookie token as
?__pt= into the internal rewrite so force-dynamic preview pages receive it
- Preview page validates token before rendering draft content; no
Next.js draftMode() — live ISR routes remain unaffected
Rename vary → variant (mechanical, codebase-wide):
- app/[vary]/ → app/[variant]/, lib/vary/ → lib/variant/,
lib/request-context/vary.ts → variant.ts, identifiers and types renamed
- Removes dead header-resolution path (resolveVaryFromHeaders / hasVaryHeaders)
that assumed a shared CDN; production topology is Next directly exposed
BFF / cache options:
- getBffCacheOptions() now synchronous, returns BffCacheOptions without
leaking isDraft into service options spreads
- ProductService and ProductCollectionService accept cacheOptions param;
loaders call getBffCacheOptions(REVALIDATE_SECONDS, { isDraft }) and
thread the result to the service so draft pages get cache: no-store
- bff-fetch-server reads variant lazily per-fetch call; correlation ID
cookie write removed (sessionStorage-only, server side generates its own)
…optimization-audit-for-core-page-templates
…e] segment
Under ISR each route segment renders independently of its parent layout.
Previously only layout.tsx called setRequestVariant, so on SPA navigation
(layout doesnt re-run) and on ISR cache hits (layout served from cache,
page re-rendered), getRequestVariant() returned undefined and
createBffFetchServer() omitted the X-Data-Source header — silently serving
all page fetches from the default backend regardless of the user's selected
data source.
Introduce initRouteContext({ variant, locale }) in
lib/request-context/route-context.ts that atomically calls setRequestLocale
and setRequestVariant(decodeVariant(variant)), mirroring the existing
setRequestLocale-per-segment pattern. Replace all direct setRequestLocale
calls across the 39 server pages/layouts with initRouteContext, adding
variant to each params destructure. Rename the requireSetRequestLocale ESLint
rule to requireInitRouteContext so the invariant is statically enforced for
any future additions. Add route-context.test.ts.
…ss feature services
…alt-variant slug resolution
…ken to local only
…core-page-templates
…ization-audit-for-core-page-templates
Extracts buildRequestContext, rewrite, handlePreviewPath, and
handleStandardPath from the monolithic proxy function. Follows the
next-intl middleware composition pattern: call intlMiddleware first,
read locale from x-middleware-rewrite, forward its response headers
via { headers: intlResponse.headers }, and explicitly set
x-next-intl-locale as a request header so getLocale() works in server
components. Required because our variant-prefix rewrite replaces
intlMiddleware's own rewrite, so its request headers don't reach
server components.
Moves from imperative setRequestVariantFromSegment() to runWithRequestVariantFromSegment() using async-local-storage. Reorganises modules: request-context/variant → lib/variant/variant-key, adds active-variant-client.ts for cookie-driven client reads.
Migrates layouts to runWithRequestVariantFromSegment() ALS model and
replaces raw headers().get('x-locale') with getLocale() from
next-intl/server.
Covers: variant survival across SPA navigation, leaked ~variant URL guard, locale switch with alt data source, malformed cookie handling, cookie-driven draft preview, cross-origin iframe preview, ISR draft isolation, root fallback shell on 404, and dot-prefixed path 404 (non-variant segments reach a clean 404, not a 500).
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Andrzej Tomala <arnimag@gmail.com>
…core-page-templates
| const locale = await getLocale() | ||
| const homeUrl = `/${rfcToUrlPrefix(locale)}` | ||
| const t = await getTranslations({ locale, namespace: 'notFound' }) | ||
| const homeUrl = `/${locale}` |
There was a problem hiding this comment.
homeUrl is built as /${locale} and then handed to the next-intl Link, which already adds the active locale itself. On a German 404 that would produce /de/de, so the "back home" button would 404 again. This could just be <Link href='/'>, matching how the header and checkout links were migrated.
| const allCrumbs = [homeCrumb, ...crumbs] | ||
|
|
||
| if (allCrumbs.length <= 1) { | ||
| if (allCrumbs.length <= 2) { |
There was a problem hiding this comment.
Changing the guard from <= 1 to <= 2 means any page with a single real crumb (home + one) now renders no breadcrumbs at all, where it used to show Home > Category. The boundary isn't covered by tests (only 0 and 3 crumbs are). If that's intended we could pin it with a test; otherwise it reads like an off-by-one and should go back to <= 1.
| const hostMatchesRedirect = | ||
| getRequestPublicHost(request) === parsedRedirectBase.host | ||
|
|
||
| if (redirectIsHttps && hostMatchesRedirect) { |
There was a problem hiding this comment.
Delivery now also depends on getRequestPublicHost() (read from the spoofable x-forwarded-host) matching the FRONTEND_URL host. When they differ on an HTTPS deploy, the route skips the secure cookie and instead puts the signed token in the URL as ?__pt= plus a non-Secure cookie — which would put the token in the address bar and logs, exactly what the now-deleted test guarded against. It's softened by the signature, short TTL, and StripPreviewToken, but an edge that forwards a non-canonical host would trip it.
There was a problem hiding this comment.
Fixed — delivery no longer looks at getRequestPublicHost() / x-forwarded-host at all. The decision is now keyed purely on the server-controlled redirect base
| * Node.js crypto out of the edge bundle. Expired or forged far-future tokens | ||
| * are treated as absent. | ||
| */ | ||
| function isDraftTokenActiveByExp(token: string): boolean { |
There was a problem hiding this comment.
The edge only checks the token's exp, not its signature (Node crypto can't run there). A cookie like <future-timestamp>.garbage would still flip isPreview on, so every page would route into /preview, fail signature validation there, and notFound() — leaving the browser stuck on 404s with no way back until cookies are cleared. We could fall through to normal routing when the token fails validation rather than dead-ending in notFound().
There was a problem hiding this comment.
The edge now also rejects tokens whose exp is further out than we ever issue (> now + DRAFT_COOKIE_MAX_AGE_SEC + 60s), so a .garbage cookie is treated as absent and never flips isPreview on. For a forged token still inside the valid window, the preview page no longer dead-ends in notFound() — it redirects to /api/draft/exit, which clears the HttpOnly cookie (a Server Component can't) and 307s back to the clean path, so the next request routes normally and the session self-heals.
| !!draftCookieToken && isDraftTokenActiveByExp(draftCookieToken) | ||
| const isPathPreview = | ||
| firstSegment === 'preview' || (firstIsLocale && segments[1] === 'preview') | ||
| const isPreview = isPathPreview || isDraftActive |
There was a problem hiding this comment.
Once an editor opens a preview link, the 24h preview_token cookie makes isDraftActive true on every request, so the proxy quietly funnels all clean URLs into the /preview subtree. That subtree only knows how to render product, collection, and content slugs, so the homepage (/ and /en both resolve to an empty path and hit notFound()) would 404, and /en/cart, /en/account, /en/checkout, /en/sign-in would fall through to <ContentPage> with a slug that doesn't exist and render an error. An editor clicking the logo or the cart mid-preview would land on a broken page until the cookie expires. We could limit cookie-driven preview to content routes, or fall back to normal routing instead of notFound() when the preview path has no match.
There was a problem hiding this comment.
The homepage now renders the draft homepage on an empty preview path instead of 404ing, and functional routes (cart, account, …) still funnel into /preview but resolve to a clean notFound() 404 via getContentPage rather than a soft error.
…core-page-templates
2 participants
No description provided.