[pull] canary from vercel:canary - #1342
Merged
Merged
Conversation
TLDR: just enable `deref_patterns` and remove the `box` syntax. And it just works `box_patterns` will be removed entirely in the next Rust nightly: rust-lang/rust#156749
The test suite has accumulated a bunch of patterns for disabling tests
that are known to fail under some configuration: `it.skip`, `if
(isNextDev) { test('skipped in dev mode', () => {}); return }`, whole
describes toggled off by checking `process.env.__NEXT_CACHE_COMPONENTS`.
These all have the same flaw: nothing tells you when the thing you
skipped starts working. The test stays disabled forever, and the
workaround it was guarding rots along with it.
React solves this with the `@gate` pragma, and this PR ports it to the
Next.js e2e harness:
```ts
// Blocked on the optimization that marks a route as fully static when
// no dynamic params are referenced in Server Components.
// @gate !cacheComponents
it('navigates to a page with a lazily-generated static param', async () => {
// body unchanged
})
```
The test still runs. If the condition is false and the test fails, the
failure is absorbed and the suite stays green. If it _passes_, the suite
fails: the gate is stale, delete it. So instead of a skip that hides a
fixed bug indefinitely, you get a CI failure the day the fix lands.
When the condition is static, the inversion is Jest's own `test.failing`
under the hood. A lazy condition isn't known until the fixture's
resolved config is read inside the body, so those tests invert at
runtime instead.
`// @force-gate <condition>` skips for real — for tests that can't even
be attempted (prefetching is disabled in dev, deploy has no local build
output, the fixture won't build under the condition), and for tests of a
new API, where the disabled state can only throw and running it proves
nothing:
```ts
// Prefetching is disabled in dev, so this suite has nothing to test.
// @force-gate prefetching
describe('segment cache prefetch scheduling', () => {
// ...
})
```
There's no staleness check in that case, so this is a judgment call:
prefer `@gate` when the off state fails for a meaningful reason — the
flag changes behavior that already exists — and `@force-gate` when the
body can only throw because the API doesn't exist. A static condition
(mode, bundler) resolves at collection time into a normal Jest skip. A
lazy condition resolves at runtime, and when a lazy force-gate on a
describe is false, we skip the fixture build entirely — that's what
makes it usable for suites whose fixtures are build-incompatible with
the condition. (One caveat: Jest has no way to skip a test that's
already running, so these report as passing with a warning in the log,
not as skipped.)
Conditions live in a hand-written registry. I considered deriving the
lazy ones from the config schema automatically, but a gate is a claim
about which dimension of the test matrix explains a failure, and I'd
rather each of those claims be spelled out with a description.
Referencing an undeclared name fails the suite at collection time, so a
typo can't silently disable a gate.
The important design decision for lazy conditions is that they read the
fixture's _resolved_ config, never `process.env`. The env var isn't the
truth: `__NEXT_CACHE_COMPONENTS=true` only applies when the fixture
doesn't set `cacheComponents` itself, and config resolution implies
flags the fixture never mentions (`cacheComponents: true` alone turns on
`experimental.ppr`). Resolution happens in a child process, because
in-process `loadConfig` would leak the fixture's `.env` files into the
Jest worker. Suites with no lazy gate never pay for any of this.
The condition expression is parsed using a small grammar (also ported
from the React repo). An expression that doesn't parse fails the suite:
```ts
// @gate mode === 'start' && !cacheComponents
// @gate !(turbopack || rspack)
```
There's also a runtime version, mirroring React's `gate(flags =>
flags.enableFoo)`, for tests that run under both states but assert
differently (and for `it.each`, where the pragma can't attach):
```ts
import { gate } from 'next-test-utils'
it('renders the fallback', async () => {
if (await gate((conditions) => conditions.cacheComponents)) {
// PPR: the fallback is part of the static shell
} else {
// fully dynamic: the fallback streams in
}
})
```
It also accepts the pragma expression language as a string: `await
gate('cacheComponents && !dev')`.
Docs are in `test/lib/gate/README.md`; `test/unit/gate/` covers the
transform, the expression language, and the runtime.
### What? Prevents Turbopack from deadlocking while resolving re-exports through an import cycle by making ECMAScript side-effect classification independent of full module analysis. The supplied async regression is added to the normal execution suite, along with a synchronous variant demonstrating that the facade/locals re-export cycle itself is sufficient to trigger the issue. ### Why? Cross-module constant analysis resolves eligible imported bindings while the importing module's analysis is still running. Re-export following asks synthesized locals modules for side effects, which delegate to their original modules. The previous fallback obtained side effects from the full `AnalyzeEcmascriptModuleResult`, so an import cycle could re-enter an already-running analysis task and close a Turbo Tasks await cycle. The actual static side-effect classifier only needs the parsed SWC program, comments, unresolved mark, directives, and module options. Reference analysis is not required. This also unblocks the export-mangling work in #97770. ### How? Side-effect classification now runs in a dedicated parse-only Turbo Task. Package/config declarations retain precedence; when none exists, the task uses `failsafe_parse()` and the existing AST classifier without resolving references or awaiting `analyze()`. `AnalyzeEcmascriptModuleResult.side_effects` and its builder state are removed because the module-side-effects fallback was its only consumer. This also eliminates unused duplicate classification during module-part analysis. `follow_reexports` and module-fragments side-effect handling remain unchanged from canary. ### Verification - `cargo test -p turbopack-tests --test execution` — 265 passed, 0 failed - `cargo test -p turbopack-ecmascript` — 525 passed, 0 failed - Focused re-export-cycle and side-effect-retention tests in both optimization modes - `cargo fmt -- --check` <!-- NEXT_JS_LLM --> <!-- fleet 4bbd5119-2442-4053-a856-f9d596924113 --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
Improves how we track accesses of runtime data in static prerenders (which affects whether a page will use static or runtime shells). Now, Instead of tracking it when the hanging promise is created, we'll track it when `then/catch/finally` is called on the promise (which we use as a signal for being used). The promise tracking required some novel work, because: - Using a proxy seems to cause `Promise.then` to show up in call stacks (we've had issues with proxies throwing off React's IO tracking before, i believe this is a similar problem) - patching `then/catch/finally` on the promise directly doesn't work, because `then` doesn't get called for native `await` The only way of doing this that i've found is a custom `Promise` subclass that tracks the access in its `then` implementation. This is what `TrackedPromise` does. To improve the debuggability, i've added some debug logs enabled by `NEXT_PRIVATE_DEBUG_RUNTIME_DATA=1` to detect when a page is deopted to runtime and what expression caused it. --- This fixes a false deopt caused by `createServerPathnameForMetadata` - when called on a route with fallback params, it would always track a fallback params access even if no icons/og images that actually use it exist. This meant that we'd never statically optimize any routes that had fallback params. After this PR, we only track a runtime data access if a metadata route that uses that params actually exists, in which case the `pathname` promise gets awaited. (as a future optimization, we should do the equivalent of vary params tracking on metadata routes to know if the params are used at all. also, the logic in `pathname.ts` is generally in need of cleanup)
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 : )