-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Improve Perf #800
Improve Perf #800
Conversation
🦋 Changeset detectedLatest commit: 5b6d8d8 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: |
Would love to see if we could get this one in - hoping to use cc. @nguyendon for vis |
@martinezdylan |
I guess I'm confused, the PR description mentions this should technically be happening now by default? Wouldn't using onStart make no difference? What we're experiencing suggests even with warm set to 20, we still are having to load / compile the routes when we first hit the endpoint. |
Because as said in the PR description, route preloading at the moment are done in the background, but warmer event return immediately, so the route are not preloaded, only NextServer itself. The Ideally you'd only use |
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.
Great PR!
My comments resolve around adding more documentation.
@@ -118,6 +118,20 @@ export interface ResolvedRoute { | |||
type: RouteType; | |||
} | |||
|
|||
/** | |||
* The route preloading behavior. Only supported in Next 15+. |
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.
Suggest:
- Add the default behavior of Next is disabled
- Add some guidelines to pick a value
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.
There is no general guidelines really, it depends so much on the deployed app that everyone should do their own testing. I'll add a comment about that
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.
a couple nits but LGTM
This PR is so good! |
All these changes only apply to Next 15+
Adds patches to the Next.js codebase to remove unused imports at runtime, improving cold start times. Early tests indicate a performance gain of 120-200 ms without any additional code changes.
Disables the default preloading of routes at startup (which was done in the background). Now by default, only the necessary routes are lazily loaded as needed.
Introduces a new configuration setting to control when routes are preloaded. The available options are:
none
: No preloading, default optionwithWaitUntil
: Preload using platform provided waitUntil - Will only occur on the first requestonWarmerEvent
: Preload on warmer events - Need to be done by the wrapper itself, onlyaws-lambda-streaming
does it for now.onStart
: Preload at startup - Be aware this is a blocking operation