refactor(core): use dynamic imports in client for next.js 16 compat#2905
refactor(core): use dynamic imports in client for next.js 16 compat#2905matthewvolk wants to merge 1 commit intocanaryfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 1dcc379 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 |
Bundle Size ReportComparing against baseline from No bundle size changes detected. |
| @@ -1,22 +1,30 @@ | |||
| import { BigCommerceAuthError, createClient } from '@bigcommerce/catalyst-client'; | |||
| import { headers } from 'next/headers'; | |||
There was a problem hiding this comment.
are these (next/headers, next/navigation, next-intl/server) the only affected paths? or will we need to worry about others later?
There was a problem hiding this comment.
Good question! Unfortunately, these Next.js API's rely on an AsyncLocalStorage store that is not resolving correctly when these API's are called in next.config.*. Any Next.js API statically imported into this file that rely on AsyncLocalStorage would re-introduce the bug... that said, Next.js opened vercel/next.js#90711 to fix this, and once released, I should be able to revert this PR 👍
|
also dumb question but should the |
…age poisoning Replace static imports of next/headers, next/navigation, and next-intl/server with dynamic import() calls inside the hooks that use them. Static imports cause these modules to be evaluated during module graph resolution when next.config.ts imports this file, poisoning the process-wide AsyncLocalStorage context. Dynamic imports defer module loading to call time, after Next.js has fully initialized.
matthewvolk
force-pushed
the
CATALYST-1808-dynamic-imports
branch
from
4ca18e2 to
1dcc379
Compare
4 participants
What/Why?
In Next.js 16 with pnpm, statically importing
next/headers,next/navigation, ornext-intl/serverat the top ofcore/client/index.tscauses AsyncLocalStorage poisoning. Whennext.config.tsimports the client module, static imports trigger the full module graph (next-intl/server→getConfig.js→RequestLocale.js→next/headers) to be evaluated during module resolution. With pnpm symlinks, this creates two separate singleton instances of thenext/headersAsyncLocalStorage, breaking request-scoped context for subsequent runtime calls.The Next.js team identified this as a bug and opened vercel/next.js#90711 to fix it upstream. However, regardless of that fix, the client module shouldn't eagerly import framework-specific APIs like
next/headersornext/navigationat the module level — these are runtime concerns that belong inside the hooks that use them. This refactor improves separation of concerns and makes the client resilient to module evaluation order, independent of any upstream fix.Solution
Replace all three static imports with dynamic
import()calls inside the hooks that use them:next-intl/server— dynamically imported ingetLocale(). During config resolution, the import succeeds butgetLocale()throws ("not supported in Client Components"), which the existing try/catch absorbs gracefully. At request time, it works normally.next/headers— dynamically imported inside thefetchOptions.cacheguard inbeforeRequest. Config-time calls pass nofetchOptions, so this never executes during config resolution.next/navigation— dynamically imported inside theBigCommerceAuthErrorcheck inonError. Config-time settings queries don't produce auth errors.Dynamic imports defer module loading to call time (after Next.js has fully initialized), avoiding the module-graph-level poisoning that static imports cause.
Scope
Only
core/client/index.tschanges.next.config.tsand all 30+ consumer files remain untouched.Testing
We added temporary instrumentation (
console.logwithNEXT_RUNTIMEvalue and call stack) togetLocale(),next.config.ts, andmiddlewares/with-routes.ts, then tested across all runtime contexts on Next.js 16.1.6 (Turbopack) by cherry-picking the Next.js 16 upgrade commits onto this branch.NEXT_RUNTIMEbehavior (key finding)We initially guarded
getLocale()with aprocess.env.NEXT_RUNTIMEcheck, expecting it to beundefinedduring config resolution. Testing revealedNEXT_RUNTIME="nodejs"in every context, including config resolution. The guard was dead code and was removed. The actual protection comes from:import()vs staticimport— defers module loading past the initialization window where poisoning occursgetLocale()failures in contexts where next-intl isn't availableRuntime context matrix
NEXT_RUNTIMEgetLocaleoutcomenext.config.tsstartup)"nodejs"defaultChannelId"edge""edge"NEXT_HTTP_ERROR_FALLBACK;404for non-matching routes → caughtGET /)"nodejs"POST /)"nodejs"GET /search?term=spray)"nodejs"GET /spray-bottle)"nodejs"GET /es-MX/spray-bottle)"nodejs"next build)"nodejs"Manual verification
Migration
No migration needed. Only
core/client/index.tswas modified. All consumer imports remain unchanged.