remove runInContext callbacks from stream-ops#90609
Merged
lubieowoce merged 1 commit intocanaryfrom Feb 27, 2026
Merged
Conversation
Collaborator
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: **400 kB** → **400 kB** ✅ -13 B80 files with content-based hashes (individual files not comparable between builds) 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.jsDiff too large to display app-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsDiff too large to display app-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsDiff too large to display app-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsDiff too large to display 📎 Tarball URL |
lubieowoce
changed the title
5 tasks
timneutkens
approved these changes
Feb 27, 2026
lubieowoce
force-pushed
the
lubieowoce/remove-run-in-context
branch
from
c523075 to
800ee35
Compare
Collaborator
Tests Passed |
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
3 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.
The
runInContextpattern that's used all overstream-opsis weird and feels very unnecessary -- we're passing in callbacks that just get invoked immediately, and all they do is callAsyncLocalStorage.run. We should just use the standardAsyncLocalStorage.runmethod outside instead. I commented about getting rid of it somewhere on the original node streams PR, but that got lost when it was was split up.I believe the original reasoning for addding it at all was that some of the node-streams implementations needed to preserve async context for callbacks -- see eg here. But if that's the motivation, then the current
runInContextpattern is wrong anyway -- sure, it preservesworkUnitAsyncStorage, but doesn't preserveworkAsyncStorageand whetever other ALSes may be present, so it's not semantically correct. If this is the goal, then the relevant callbacks should usebindSnapshot(i.e.AsyncLocalStorage.bind) instead, because it preserves the entire context, not just one ALS.(Note that technically the
runInContextpattern is salvageable if we just always passAsyncLocalStorage.snapshot(), but then i see no reason why the caller should have to do that.AsyncLocalStorage.bindis cleaner)Note that this'll require changes in #89859