refactor: make stage advancing logic more generic#94349
Merged
Conversation
Contributor
Tests PassedCommit: 2eeac8e |
Contributor
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (8 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsfailed to diffapp-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsfailed to diff📎 Tarball URLCommit: 2eeac8e |
lubieowoce
force-pushed
the
lubieowoce/stage-controller-generic
branch
2 times, most recently
from
d515588 to
bc5f88c
Compare
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
lubieowoce
force-pushed
the
lubieowoce/stage-controller-generic
branch
from
bc5f88c to
a302387
Compare
This was referenced Jun 2, 2026
lubieowoce
force-pushed
the
lubieowoce/stage-controller-generic
branch
from
a302387 to
0f5dc1f
Compare
lubieowoce
force-pushed
the
lubieowoce/stage-controller-generic
branch
from
0f5dc1f to
2eeac8e
Compare
gnoff
approved these changes
Jun 2, 2026
| const endStage = getNextStage(RenderStage.Static) | ||
| stageController.onStage(endStage, () => { | ||
| reader.cancel() | ||
| }) |
Contributor
There was a problem hiding this comment.
Is this pattern common? Should we model this as afterStage and let you pass in Static and it fires after you transition past this stage as opposed to on entering the next stage?
Member
Author
There was a problem hiding this comment.
i haven't seen it elsewhere (other than the node/web forks). but also importantly there's an implicit assumption here that we'll reach a stage after the target stage at all. here we know we will, because it's working off of a full staged render, but in other codepaths, if we're doing a prerender, we might not reach the target stage at all and would need to deal with that somehow. we'd probably need to to call something at the end of the task where we do this stage. anyway i wanted to avoid making API design decisions on that, so i didn't make this generic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #93801, we'll be adding 4 new stages.
StagedRenderingControlleralready contains a lot of duplication around advancing stages, and i want to avoid adding to that, so this PR does some cleanup.Previously, for a stage, we had
resolve<name>Stage- triggers listener callbacks and resolves the promise for the stage<name>StageListeners- added viaonStage, triggered when advancing to the stage but before resolving the promise<name>StagePromise- resolved when we enter the stage (or rejected on abort/abandon)<name>EndTimethese behaviors are all now packaged up into
StageTrigger, which can be "fired" (when advancing to a stage) or "cancelled" (when aborting/abandoning) and tracks the time.In addition, we now have
RENDER_STAGE_ADVANCE_ORDER, an array that defines the order in which advanceable stages (i.e. excluding Before/Abandoned) should happen. We can loop through this to advance stages without relying on a huge switch statement with fallthroughs. Re-working the logic into this shape will be handy in the future, when we want each route param to have its own stage, and the set of stages is no longer statically known.