-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
cache Mongoose connections between hot reloads #23321
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @vkarpov15 for the PR! Requesting changes based on my suggestion above 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took the opportunity to update the readme because the deploy button is broken and it now looks good to me. Thank you @vkarpov15 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes - No, I get hundreds of unrelated errors like: ``` build/babel/plugins/next-page-config.ts(8,28): error TS2307: Cannot find module 'next/types'. build/entries.ts(11,32): error TS2307: Cannot find module '@next/env'. build/index.ts(1,31): error TS2307: Cannot find module '@next/env'. build/index.ts(9,10): error TS2305: Module '"next/dist/compiled/nanoid/index.cjs"' has no exported member 'nanoid'. ``` Re: Automattic/mongoose#9987, the current Mongoose example doesn't do a good job of persisting state between hot reloads. Not sure why Lambda doesn't keep the same `import mongoose` between reloads, but it doesn't seem to. This approach lines up more closely with [Mongoose's Lambda docs](https://mongoosejs.com/docs/lambda.html) and the existing `with-mongodb` example.
For some reason I'm not quite sure of, the caching code in this PR makes my lambda functions unpredictably fail around 50% lf the time on AWS using nextjs-serverless. Unfortunately, I don't have error logs (because I couldn't find a reliable logging solution). It took me about 2 hours of guessing to isolate the root cause. Sorry for the lack of details. |
Bug
fixes #number
Feature
fixes #number
Documentation / Examples
Re: Automattic/mongoose#9987, the current Mongoose example doesn't do a good job of persisting state between hot reloads. Not sure why Lambda doesn't keep the same
import mongoose
between reloads, but it doesn't seem to. This approach lines up more closely with Mongoose's Lambda docs and the existingwith-mongodb
example.