[pull] canary from vercel:canary#1123
Merged
Merged
Conversation
### What? Adopt `[scattered_collect`](https://docs.rs/scattered-collect/latest/scattered_collect/) for collecting static rcstr values. ### Why? Built by our good friend @mmastrac to solve some of our problems. Replaces inventory with a simpler linker driver approach. Basically inventory works using constructor functions to build a linked list, then iterating follows the list. `scattered_collect` on the otherhand uses a linker section for the data, so it all gets moved to a dedicated part of the binary and bingo, a slice!. This means we don't need `init` time logic. This is preparation for migrating other uses of `inventory` just to prove it out. Which was a good idea because we found and fixed a bug! See https://github.com/mmastrac/linktime/releases/tag/link-section-0.18.2 ## Wasm scatter_collect supports wasm, but requires us to register a 'section reader' function with the runtime, see https://docs.rs/link-section/latest/link_section/#wasm. The instructions are staightforward but a little tricky since we rely on `wasm-bindgen` to constrct the `Wasm.Module` instance and it is encapsulated. So to 'inject' that function we would need to patch some of the bootstrapping. Given that this registry is only an optimization for deserialization which never happens in a wasm build i just flagged it all off.
The client sends the dev-only `x-nextjs-request-id` and `x-nextjs-html-request-id` headers so the server can route debug information back to the originating request. They are internal plumbing, like the flight headers, but unlike the flight headers they were never stripped from the headers exposed to userland `headers()`. This change strips them in `getHeaders` alongside the flight headers. The server still reads them from the raw request headers, not from the sealed userland copy, so the debug channel is unaffected.
Private caches were never stored in a cache handler, so every reload in `next dev` re-ran them from scratch and they registered as a cache miss on each load. This change persists `'use cache: private'` entries in development in a dedicated built-in in-memory handler so that warm reloads are fast. The handler is gated on `process.env.__NEXT_DEV_SERVER` and is kept out of the kind-keyed handlers map so it can never be replaced by a user-configured `default` handler: private cache entries can hold data specific to the incoming request (for example, derived from its cookies or headers) and must never reach a remote or otherwise persistent handler. Production keeps private caches non-persisted as before. The coarse cache-handler key for a dev private cache is scoped by the request's cookies and headers so entries for requests with different request data don't collide. It excludes Next-internal cookies that aren't application data (the HMR refresh hash, already part of the cache key, and the instant-navigation cookie, which toggles while a navigation lock is held) and the transport and content-negotiation headers that vary between otherwise-equivalent requests (a browser reload adds `cache-control`, and `accept` and `sec-fetch-*` differ between an HTML navigation and an RSC request). Keying by only the cookies and headers a cache actually reads is left as a follow-up; read root params are already tracked that way, the same as for public caches. The cache life is forced to `revalidate: 0` with a 5-minute `expire`, so each read serves the stale entry immediately and warms a fresh one in the background through the existing stale-while-revalidate path. Cross-request deduplication now applies to private caches in development too, so concurrent requests with identical request data share a single fill; it remains skipped in production where request-specific data must not be shared across requests. To make this deterministic, `saveToCacheHandler` resolves the metadata a cross-request joiner awaits only after the entry has been written to the handler, so the joiner finds it when re-reading its recomputed key. This closes a pre-existing race in that path, present for public caches too but never surfaced by their cross-request test, where the joiner's metadata could resolve before the handler write had landed. The defensive invariant that rejected reading a private entry from a handler is removed: it only existed to narrow `cacheContext.kind` for a code path that no longer needs it, and dev now legitimately reads persisted private entries while production never registers a private handler to read from.
# What This uses `scatter_collect` from @mmastrac to register all values, traits and value impls of traits. now all data can be consistently accessed when constructing the VALUEs registry, eliminating a number of racy access opportunities. # Why Scatter collect uses link sections to gather registries at link time, this resolve some inherent brittleness in constructor functions since the relative execution order is **not guaranteed** and it is also not even possible to _detect_ execution that is too early (early access to an `inventory` collection will miss some entries silently) We have a single report that appears to be due to a race like this ``` web:dev: thread 'tokio-rt-worker' (41574444) panicked at turbopack/crates/turbo-tasks/src/macro_helpers.rs:274:13: web:dev: no trait impl registered for value type turbopack_core::module_graph::ModuleGraphImportTracer web:dev: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` I do not entirely understand how this is possible since constructors should complete prior to the first napi entry point (other than `napi::module_init` which is itself a constructor). Our initialization logic is fundamentally brittle since it relies on * an `inventory` of all value types * an `inventory `of all trait types * a set of ctor functions to register trait implementations (basically (value, trait) pairs) All of these need to be initialized/invoked before the `VALUES` registry is accessed, but the above panic implies that the ctor function for `ModuleGraphImportTracer` to register its implementation of `ImportTracer` hasn't occurred yet. By moving the registies to link time we completely eliminate all startup races since the linker does all the 'gathering', now the only possible concern is that we attempt to downcast a trait prior the the `VALUES` registry completing which does not appear to be possible. # Binary size impact Measured on `next-swc.darwin-arm64.node` built with `pnpm --filter @next/swc build-native-release` (release profile), this branch vs `canary`. Stripped = `strip -x`; gzip = `gzip -9` of the stripped binary. | Metric | branch | canary | Δ | Δ% | |---|---:|---:|---:|---:| | **raw** | 122,879,888 B | 124,184,976 B | **−1,305,088 B** | **−1.051%** | | **stripped** | 83,505,264 B | 84,127,232 B | **−621,968 B** | **−0.739%** | | **gzip+stripped** | 28,800,547 B | 28,925,679 B | **−125,132 B** | **−0.433%** | The addon shrinks on every metric. Replacing the generated per-impl registration functions (and their `ctor`/`inventory` machinery) with link-time scattered collection removes real code/data — ~622 KB even after stripping — and laying the registries out contiguously at link time compresses better, so the gzip win holds up too. <!-- NEXT_JS_LLM_PR -->
The 'request' codepath in params/searchParams was forked `process.env.NODE_ENV === 'development'`, but that's a leftover from when dev was the only place where we did staged rendering. It didn't account for conditions like `--debug-prerender`, and now makes it generally confusing to reason about. This PR simplifies the flow so that renders that care about stages are clearly distinguished, and dev is just a minor variation within that (generally adding some extra warnings via proxies)
When we send a `next-router-prefetch: 3` request, we now track if unblocking session data produced new content. Exposed as `u?: Promise<boolean>` on the navigation response.
Distinguish an absent postponed state from an empty one so cold RDC resume requests perform a full dynamic RSC render instead of being rejected or treated as revalidation.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )