Add override for esbuild plugin #656
Conversation
|
commit: |
| @@ -28,7 +28,11 @@ import { | |||
| overrideNextjsRequireHooks(NextConfig); | |||
| applyNextjsRequireHooksOverride(); | |||
| //#endOverride | |||
|
|
|||
| //#override cacheHandlerPath | |||
| const cacheHandlerPath = require.resolve("./cache.cjs"); | |||
There was a problem hiding this comment.
After some thought, i don't see how setting the NEXT_CACHE_HANDLER_PATH env would benefit us that much, at least not here.
The only way this would be useful would be at build time so that the cache is available during next build, but then it would mean that we'll have to either include open-next in the bundle (or at least a precompiled version of the cache in the node_modules) or still override it at build time like we do today.
It also means that both the tag and incremental cache would need to be available at build time
There was a problem hiding this comment.
Here again we can not delete this but we should override or it will break the build.
There was a problem hiding this comment.
Right, you could replace it using the replacement plugin instead of deleting
| @@ -28,7 +28,11 @@ import { | |||
| overrideNextjsRequireHooks(NextConfig); | |||
There was a problem hiding this comment.
@vicb These 2 uses require.resolve and are present on next >=13.4.13 and next <=13.5.1
|
@vicb PTAL, with these changes you should be able to just add these 2 overrides in the deletes part of the replacement plugin and remove the OpenNext patch |
| @@ -19,7 +19,9 @@ import { requestHandler, setNextjsPrebundledReact } from "./util"; | |||
| // This is used to identify requests in the cache | |||
| globalThis.__openNextAls = new AsyncLocalStorage(); | |||
|
|
|||
| //#override patchAsyncStorage | |||
There was a problem hiding this comment.
Long term, we should not delete it but have a cloudflare specific impl.
Maybe we could then override instead of deleting but at the same time the temporary patch in cloudflare is good enough before we have a long term solution.
WDYT?
There was a problem hiding this comment.
This one will hopefully be removed in the future in OpenNext as well.
Once this PR vercel/next.js#72082 gets merged it will not be necessary anymore.
I just have to figure out how to test with the new use cache stuff from next
| @@ -28,7 +28,11 @@ import { | |||
| overrideNextjsRequireHooks(NextConfig); | |||
| applyNextjsRequireHooksOverride(); | |||
| //#endOverride | |||
|
|
|||
| //#override cacheHandlerPath | |||
| const cacheHandlerPath = require.resolve("./cache.cjs"); | |||
There was a problem hiding this comment.
Here again we can not delete this but we should override or it will break the build.
I'll change it to draft then |
No description provided.