fix: make patchAsyncStorage work with non-mutable node:module modules
#730
Conversation
🦋 Changeset detectedLatest commit: 35be04f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
commit: |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
|
|
||
| const resolveFilename = mod._resolveFilename; | ||
|
|
||
| export function patchAsyncStorage() { | ||
| mod._resolveFilename = (( | ||
| const _resolveFilename = (( |
There was a problem hiding this comment.
I don't think this is correct path on workers.
_resolveFilename is not used on workers (it throws) as we do not use dynamic require - requires/imports are resolved at build time.
There was a problem hiding this comment.
Maybe we can use a build time patch instead, replacing the Next version with the patch.
That should be pretty easy to do via the onResolve hook of an ESBuild plugin
There was a problem hiding this comment.
ah ok I see, yes that's a good call thanks 🙂👍
I'm happy to close the PR if we don't see the point in this then (however it is a bit too bad that aws does something that then we need to un-do, I wonder if that could be solved somehow at some point 😕)
|
Thanks for having a look @conico974 🫶 But I'm going to close the PR as per @vicb's suggestion, the fix should probably be done in our adapter PS: if you were to add an e2e test please do let me know as I would be interested to see the effect of this function, if this function is definitely necessary for fetch cache I wonder why my change was working in our adapter (unless our adapter was throwing but swallowing the errors?! 🤔) |
This change allows
patchAsyncStorageto work with the cloudflare adapter allowing us to get rid of the code that comments the function outThe problem with the current patch is that it updates
mod._resolveFilenamewhich doesn't work in the cloudflare adapter since the worker gets build by esbuild making the exports of the module (which we polyfill using unenv) non-settable:Warning
Disclamer: I did test the Cloudflare adapter with this change and this does allow us to use
patchAsyncStoragewithout getting the runtime error anymore, I could however not see any other difference in the app I tested this with (theapp-routerapp), if anyone would have suggestions on how/if I can check some effects that the patch has that would be very much appreciated 🙏