-
Notifications
You must be signed in to change notification settings - Fork 180
fix(cache): add null check for globalThis.openNextConfig #1042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Adds optional chaining when accessing globalThis.openNextConfig to prevent TypeError during Next.js 16 build phase when using the Adapters API. The cache handler can be instantiated during SSG/prerendering before openNextConfig is initialized by the runtime handlers.
🦋 Changeset detectedLatest commit: afe0d93 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Is this PR AI generated? |
@vicb Is the usage of AI tooling a problem? The goal is to be comprehensive in the tests, documentation, PR description, and completeness of the solution. |
|
I won't speak on behalf of the cloudflare side, but on the aws side, there's nothing wrong w/ using AI. Thanks for the PR! |
|
Might be worth investigating if we can move the instantiation of the openextConfig before so it's available at build time to avoid the type checking. I believe we had a similar issue a few months ago where we fixed some out of order function calls. |
I can take a look to see if that's a possibility instead of just null checking, if it's possible I'll close this and open a new PR |
|
I dug into this to see if we could reorder things to avoid the optional chaining. Unfortunately it's not possible due to how ES module evaluation works. The issue is that The config is set inside I traced through the execution order:
So the cache class is defined during step 2, but the config isn't available until step 3. The only alternatives would be build-time injection (loses runtime config flexibility, and I found an earlier commit where this concept was actually removed The only time the null chaining is needed is during the brief module evaluation phase at cold start. After that the config is guaranteed to be set before any requests are handled. |
|
Ah ok, thanks for the investigation. |
What?
Adds null checks for
globalThis.openNextConfigin the cache handler to prevent TypeError during Next.js 16 build phase.Why?
When using the Next.js 16 Adapters API, the cache handler class is instantiated during SSG/prerendering before
globalThis.openNextConfigis initialized by the runtime handlers. This causes:How?
Changed all occurrences of:
To:
Fixed 4 locations in
packages/open-next/src/adapters/cache.ts:get()method (line 48)set()method (line 207)revalidateTag()method (line 325)updateTagsOnSet()method (line 433)Testing
globalThis.openNextConfigis undefinedType of Change
Related Issues
This is a blocker for the Adapters API work in opennextjs-cloudflare.