diff --git a/.agents/skills/gate-tests/SKILL.md b/.agents/skills/gate-tests/SKILL.md new file mode 100644 index 000000000000..c6d25be694a0 --- /dev/null +++ b/.agents/skills/gate-tests/SKILL.md @@ -0,0 +1,194 @@ +--- +name: gate-tests +description: > + How to use the `@gate` / `@force-gate` test directives instead of `it.skip` + or fake-green skip patterns. Use when a test is known-failing under some + test-matrix dimension (dev mode, a bundler, an experimental flag like + cacheComponents), when converting `if (isNextDev) return` guards or + env-var `describe.skip` branches, when adding a condition to + test/lib/gate/conditions.ts, or when keying a fixture's experimental flag + on a __NEXT_TEST_AXIS letter. Covers directive choice, condition tiers, + the test-axis fixture pattern, pitfalls, and verification commands. +user-invocable: false +metadata: + internal: true +--- + +# Gating tests with `@gate` / `@force-gate` + +Full reference: [`test/lib/gate/README.md`](../../../test/lib/gate/README.md). +This skill is the decision guide: which directive to reach for, the standard +conversion patterns, and how to verify. + +## Never write these — gate instead + +| Anti-pattern | Replacement | +| ---------------------------------------------------------------------- | --------------------------------------------------------------------------- | +| `it.skip('...')` for a known failure | `// @gate ` (or `@gate FIXME` if no condition explains it) | +| `if (isNextDev) { test('skipped in dev mode', () => {}); return }` | `// @force-gate prefetching` (or `!dev`) on the `describe` | +| `(flagEnabled ? describe.skip : describe)(...)` keyed on `process.env` | `// @force-gate ` (lazy) on the `describe` | +| Duplicating a fixture directory per flag state | one fixture keyed on `__NEXT_TEST_AXIS` + a `@gate`/`@force-gate` | +| Branching expectations on `process.env.__NEXT_CACHE_COMPONENTS` | `if (await gate((c) => c.cacheComponents))` (`gate` from `next-test-utils`) | + +The skip patterns are fake-greens: nothing tells you when the bug they hide is +fixed. `@gate` still runs the body and fails the suite the day the "known +failure" starts passing, so stale workarounds get deleted instead of rotting. + +## Choosing the directive + +Ask what kind of difference you're encoding: + +1. **A behavior change — both states assert something meaningful.** Don't + gate the test at all: fork inside the body with the runtime `gate()` — + same condition registry, no inversion — which pinpoints exactly what + differs, and also covers `it.each`, where a pragma cannot attach: + `if (await gate((c) => c.cacheComponents)) { ... } else { ... }`. It + mirrors React's `gate(flags => ...)`; a pragma expression string works + too (`await gate('cacheComponents && !dev')`). A suite-level pragma is + too coarse here — it hides _what_ is different between the states. +2. **A flag that changes the behavior of existing surface** + (`cacheComponents`, `optimisticRouting`) **and the suite is written for one + state.** `// @gate ` on the test or `describe`. The body runs; a + false condition inverts the expectation (failure absorbed, a pass fails as + "stale gate"). The off state fails for a meaningful reason — the behavior + differs — so a pass is real information: the gate is stale, delete it. +3. **A new API — the off state proves nothing.** Typically + `// @force-gate ` (lazy) on the `describe`. An API that throws when + its flag is off — or is inert, like `useOffline()`, which compiles to a + hook that always reports online — can only fail vacuously (often slowly, + by timing out), and browser e2e time is considerable, so skip the run + (and the fixture build) instead of paying for it. Working example: + `test/e2e/app-dir/use-offline/`. This is discretion, not a rule: when the + flag changes behavior the suite can observe, the off state is meaningful + and `@gate` buys the staleness check. +4. **`@force-gate ` also when running the body is impossible**, not + merely failing: prefetching is off in dev, deploy has no local build + output, the fixture cannot even build under the condition. + - Static condition (`!dev`, `bundler`…) → real Jest `○ skipped` at + collection. + - Lazy condition on a `describe` → the fixture **build is skipped** when + false; tests report passed-with-`⚠ skipped by @force-gate` (Jest cannot + skip at runtime). Build-skipping covers `start`/`dev` suites where + `nextTestSetup` owns the build — not `skipStart` suites, not deploy. +5. Pragmas stack: a common pair is a static `// @force-gate prefetching` plus + a lazy `// @gate ` on the same `describe`. + +### Is the off-state run worth its cost? + +Browser e2e time is not free, so weigh what the gated-off run buys. For a +behavior flag it usually replaces a run that was already being paid for — a +fixture that pins its flags runs identically with and without the axis set, +so keying the flag on an axis converts a redundant duplicate into coverage — +and it is what proves a pass isn't vacuous: a test that passes with the +feature off wasn't testing the feature. Absorbed failures also fail fast, so +the off-state run is cheaper than it sounds. For a new API the calculus +flips: the off state can only throw, which proves nothing, so use a lazy +`// @force-gate ` on the `describe` — the fixture build is skipped +too, so the off state costs almost nothing. + +## Conditions + +Every name in a pragma must be declared in `test/lib/gate/conditions.ts` +(typos fail the suite at collection). Two tiers: + +- **static** — the run's shape: `dev`, `start`, `deploy`, `mode`, `turbopack`, + `rspack`, `webpack`, `bundler`, `react18`, `wasm`, `ci`, plus the + always-false `FIXME`/`TODO`. `prod` and `prefetching` are semantic aliases + for `!dev` — prefer the name that states _why_ the suite cannot run. +- **lazy** — a predicate over the fixture's _resolved_ `next.config` + (`cacheComponents`, `ppr`, `useOffline`, `output`, …). + +Adding one is a two-line change; follow the guidance at the top of +`conditions.ts`. The rule that matters: **lazy conditions read the resolved +config, never `process.env`** — env vars don't survive config resolution +(`__NEXT_CACHE_COMPONENTS` only applies when the fixture doesn't set +`cacheComponents` itself, and resolution implies flags the fixture never +mentions). + +## Pattern: cover both states of an experimental flag + +Instead of pinning a flag on (which makes the plain and axis runs identical), +key it on a test axis and gate the suite. Axes are lettered (`A`, `B`, …) — +a fixed enumeration, not a boolean and not a sharding bucket. Key the flag so +it is **enabled by default** — then the suite exercises the feature in plain +local runs with no special env, and the axis run covers the off state: + +```js +// next.config.js — pin every dimension except the one under test +const nextConfig = { + cacheComponents: true, + experimental: { + concurrentRouterQueue: process.env.__NEXT_TEST_AXIS !== 'A', + }, +} +``` + +```ts +// @gate concurrentRouterQueue +it('fails loudly on link navigation', async () => { ... }) +``` + +The plain run exercises the feature; the axis-A run covers the off state — +the gated tests are expected to fail there, and the suite fails the day they +start passing. Working example: `test/e2e/app-dir/concurrent-router-queue/` +(tests whose expectations hold in both states stay ungated). The same keying +pairs with a lazy `@force-gate` when the off state proves nothing — +`test/e2e/app-dir/use-offline/` — which skips the redundant axis run (build +included) instead of covering it. Axis `A` aliases `__NEXT_CACHE_COMPONENTS` +for now (see `scripts/run-jest.sh`) — fine, because these fixtures pin +`cacheComponents` explicitly, so that run's env default is a no-op for them. + +**Keep exactly one flag varying per fixture.** A red shard must attribute to a +single dimension. + +## Pitfalls + +- A pragma the transform can't attach is a **hard error**: a blank line + between pragma and `it(`, `it.each`/`it.failing`, or a pragma inside a + JSDoc block. Prose comments must not begin with `@gate`. A pragma on a + skipped test (`it.skip`, `xit`, …) errors as ambiguous — remove the skip or + the pragma. A skip without a pragma is respected. +- A `describe`-level gate does not reach `it.each` tests. +- Gated-false bodies that fail by _stalling_ waste the full Jest timeout — + and under a lazy gate they fail the suite anyway (the runtime inversion + only absorbs thrown errors; a static gate rides Jest's native + `test.failing`, which does absorb timeouts). Bodies that fail via `retry()` + timeouts also make the off-state run slow; a fast first assertion is worth + having. +- Failures cascade in the off state: an absorbed failure mid-body skips the + body's cleanup (e.g. a browser context left offline), so later tests may + fail for cascade reasons. Acceptable for a tripwire, but don't puzzle over + the individual failure messages in a gated-off run. +- `afterEach` failures (e.g. redbox matchers) are not gated — only the body is. +- `jest.retryTimes(1)` on non-dev CI means a _flaky_ gated-false test passes + whenever it happens to fail; the tripwire is only deterministic for + deterministic tests. +- Gated titles are unchanged in the Jest output; the + `⚠ gated test failed as expected` log line is the only signal. +- `pragma-transform.js` bails out early on files containing neither `@gate` + nor `@force-gate` as substrings — keep both checks if you touch it. + +## Verify a gated suite in every state it can run in + +```sh +# plain run (flag on): expect normal passes, no warnings +NEXT_SKIP_ISOLATE=1 pnpm test-start-webpack test/e2e/app-dir//.test.ts + +# axis run (flag off): expect `⚠ gated test failed as expected (@gate …)` +__NEXT_TEST_AXIS=A NEXT_SKIP_ISOLATE=1 pnpm test-start-webpack test/e2e/app-dir//.test.ts + +# dev (static @force-gate !dev): expect `○ skipped` at collection, no fixture boot +NEXT_SKIP_ISOLATE=1 pnpm test-dev-webpack test/e2e/app-dir//.test.ts +``` + +A suite with a lazy `@force-gate` on the `describe` should additionally show +`skipping build` behavior (no `next build`) in the state where the condition +is false. + +Unit tests for the infrastructure itself: `pnpm test-unit test/unit/gate/`. + +## Related skills + +- `$flags` — adding the experimental flag itself (config-shared, schema, + define-env) +- `$router-act` — the prefetch-timing patterns most gated suites also use diff --git a/AGENTS.md b/AGENTS.md index 4642ba43525f..1b82298679e6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -438,6 +438,7 @@ Use skills for conditional, deep workflows. Keep baseline iteration/build/test p - `$react-sync` - build a local React checkout and sync it into Next.js for testing - `$runtime-debug` - runtime-bundle/module-resolution regression reproduction and verification - `$next-rspack` - @next/rspack-core and @next/rspack-binding maintenance (rspack/ directory) +- `$gate-tests` - `@gate`/`@force-gate` test directives: replacing `it.skip`/fake-green skips, conditions, variant-shard fixtures - `$authoring-skills` - how to create and maintain skills in `.agents/skills/` ## Context-Efficient Workflows diff --git a/crates/next-custom-transforms/src/lib.rs b/crates/next-custom-transforms/src/lib.rs index 0e2a59fd7283..2ed9719c32a0 100644 --- a/crates/next-custom-transforms/src/lib.rs +++ b/crates/next-custom-transforms/src/lib.rs @@ -28,7 +28,7 @@ DEALINGS IN THE SOFTWARE. #![recursion_limit = "2048"] #![deny(clippy::all)] -#![feature(box_patterns)] +#![feature(deref_patterns)] #![feature(arbitrary_self_types)] #![feature(arbitrary_self_types_pointers)] diff --git a/crates/next-custom-transforms/src/transforms/import_analyzer.rs b/crates/next-custom-transforms/src/transforms/import_analyzer.rs index fcc553951c78..e7a132fb2af4 100644 --- a/crates/next-custom-transforms/src/transforms/import_analyzer.rs +++ b/crates/next-custom-transforms/src/transforms/import_analyzer.rs @@ -34,7 +34,7 @@ impl ImportMap { } Expr::Member(MemberExpr { - obj: box Expr::Ident(obj), + obj: Expr::Ident(obj), prop: MemberProp::Ident(prop), .. }) => { diff --git a/crates/next-custom-transforms/src/transforms/optimize_server_react.rs b/crates/next-custom-transforms/src/transforms/optimize_server_react.rs index 6a503edf85f0..0c099d1c0e9b 100644 --- a/crates/next-custom-transforms/src/transforms/optimize_server_react.rs +++ b/crates/next-custom-transforms/src/transforms/optimize_server_react.rs @@ -51,7 +51,7 @@ fn effect_has_side_effect_deps(call: &CallExpr) -> bool { if let Expr::Array(arr) = &*call.args[1].expr { for elem in arr.elems.iter().flatten() { if let ExprOrSpread { - expr: box Expr::Call(_), + expr: Expr::Call(_), .. } = elem { @@ -137,7 +137,7 @@ impl Fold for OptimizeServerReact { fn fold_expr(&mut self, expr: Expr) -> Expr { if let Expr::Call(call) = &expr { - if let Callee::Expr(box Expr::Ident(f)) = &call.callee { + if let Callee::Expr(Expr::Ident(f)) = &call.callee { // Mark `useEffect` as DCE'able if let Some(use_effect_ident) = &self.use_effect_ident && &f.to_id() == use_effect_ident @@ -154,7 +154,7 @@ impl Fold for OptimizeServerReact { return wrap_expr_with_env_prod_condition(call.clone()); } } else if let Some(react_ident) = &self.react_ident - && let Callee::Expr(box Expr::Member(member)) = &call.callee + && let Callee::Expr(Expr::Member(member)) = &call.callee && let Expr::Ident(f) = &*member.obj && &f.to_id() == react_ident && let MemberProp::Ident(i) = &member.prop @@ -179,8 +179,8 @@ impl Fold for OptimizeServerReact { if let Pat::Array(array_pat) = &decl.name && array_pat.elems.len() == 2 - && let Some(box Expr::Call(call)) = &decl.init - && let Callee::Expr(box Expr::Ident(f)) = &call.callee + && let Some(Expr::Call(call)) = &decl.init + && let Callee::Expr(Expr::Ident(f)) = &call.callee && let Some(use_state_ident) = &self.use_state_ident && &f.to_id() == use_state_ident && call.args.len() == 1 diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 277532b658df..185a15c2a5e5 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -1567,15 +1567,15 @@ impl VisitMut for ServerActions { let old_current_export_name = self.current_export_name.take(); match n { - PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp { + PropOrSpread::Prop(Prop::KeyValue(KeyValueProp { key: PropName::Ident(ident_name), - value: box Expr::Arrow(_) | box Expr::Fn(_), + value: Expr::Arrow(_) | Expr::Fn(_), .. })) => { self.current_export_name = None; self.arrow_or_fn_expr_ident = Some(ident_name.clone().into()); } - PropOrSpread::Prop(box Prop::Method(MethodProp { key, .. })) => { + PropOrSpread::Prop(Prop::Method(MethodProp { key, .. })) => { let key = key.clone(); if let PropName::Ident(ident_name) = &key { @@ -1603,7 +1603,7 @@ impl VisitMut for ServerActions { if !self.in_module_level && self.should_track_names - && let PropOrSpread::Prop(box Prop::Shorthand(i)) = n + && let PropOrSpread::Prop(Prop::Shorthand(i)) = n { self.names.push(Name::from(&*i)); self.should_track_names = false; @@ -1686,7 +1686,7 @@ impl VisitMut for ServerActions { } fn visit_mut_call_expr(&mut self, n: &mut CallExpr) { - if let Callee::Expr(box Expr::Ident(Ident { sym, .. })) = &mut n.callee + if let Callee::Expr(Expr::Ident(Ident { sym, .. })) = &mut n.callee && (sym == "jsxDEV" || sym == "_jsxDEV") { // Do not visit the 6th arg in a generated jsxDEV call, which is a `this` @@ -2837,7 +2837,7 @@ impl VisitMut for ServerActions { (&attr.value, &attr.name) { match &container.expr { - JSXExpr::Expr(box Expr::Arrow(_)) | JSXExpr::Expr(box Expr::Fn(_)) => { + JSXExpr::Expr(Expr::Arrow(_)) | JSXExpr::Expr(Expr::Fn(_)) => { self.arrow_or_fn_expr_ident = Some(ident_name.clone().into()); } _ => {} @@ -2852,7 +2852,7 @@ impl VisitMut for ServerActions { let old_current_export_name = self.current_export_name.take(); let old_arrow_or_fn_expr_ident = self.arrow_or_fn_expr_ident.take(); - if let (Pat::Ident(ident), Some(box Expr::Arrow(_) | box Expr::Fn(_))) = + if let (Pat::Ident(ident), Some(Expr::Arrow(_) | Expr::Fn(_))) = (&var_declarator.name, &var_declarator.init) { if self.in_module_level @@ -3301,7 +3301,7 @@ fn has_body_directive(maybe_body: &Option) -> (bool, bool) { for stmt in body.stmts.iter() { match stmt { Stmt::Expr(ExprStmt { - expr: box Expr::Lit(Lit::Str(Str { value, .. })), + expr: Expr::Lit(Lit::Str(Str { value, .. })), .. }) => { if value == "use server" { @@ -3440,7 +3440,7 @@ impl DirectiveVisitor<'_> { match stmt { Stmt::Expr(ExprStmt { - expr: box Expr::Lit(Lit::Str(Str { value, span, .. })), + expr: Expr::Lit(Lit::Str(Str { value, span, .. })), .. }) => { if value == "use server" { @@ -3575,8 +3575,8 @@ impl DirectiveVisitor<'_> { } Stmt::Expr(ExprStmt { expr: - box Expr::Paren(ParenExpr { - expr: box Expr::Lit(Lit::Str(Str { value, .. })), + Expr::Paren(ParenExpr { + expr: Expr::Lit(Lit::Str(Str { value, .. })), .. }), span, @@ -3668,7 +3668,7 @@ impl VisitMut for ClosureReplacer<'_> { fn visit_mut_prop_or_spread(&mut self, n: &mut PropOrSpread) { n.visit_mut_children_with(self); - if let PropOrSpread::Prop(box Prop::Shorthand(i)) = n { + if let PropOrSpread::Prop(Prop::Shorthand(i)) = n { let name = Name::from(&*i); if let Some(index) = self.used_ids.iter().position(|used_id| *used_id == name) { *n = PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp { diff --git a/jest.config.js b/jest.config.js index 19ab0ab0485f..746ae096c73e 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,4 +1,5 @@ const nextJest = require('next/jest') +const { withGateTransformer } = require('./test/lib/gate/jest-transformer') const createJestConfig = nextJest() @@ -89,4 +90,11 @@ if (enableTestReport) { } // createJestConfig is exported in this way to ensure that next/jest can load the Next.js config which is async -module.exports = createJestConfig(customJestConfig) +const createConfig = createJestConfig(customJestConfig) + +module.exports = async function createConfigWithGates() { + // `withGateTransformer` chains the `@gate` pragma rewrite in front of the + // SWC transformer that `next/jest` configured, keeping next/jest's SWC + // options as the single source of truth. See test/lib/gate/. + return withGateTransformer(await createConfig()) +} diff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts index dfbf51a64573..45fef9364fa4 100644 --- a/packages/next/src/server/app-render/dynamic-rendering.ts +++ b/packages/next/src/server/app-render/dynamic-rendering.ts @@ -355,7 +355,7 @@ export function abortAndThrowOnSynchronousRequestDataAccess( // promise (see makeRuntimeHangingPromise). Unlike // `abortOnSynchronousPlatformIOAccess`, which aborts a runtime prerender // all the same and therefore must not record anything. - trackRuntimeDataAccessed(prerenderStore) + trackRuntimeDataAccessed(prerenderStore, expression) const prerenderSignal = prerenderStore.controller.signal if (prerenderSignal.aborted === false) { diff --git a/packages/next/src/server/dynamic-rendering-utils.test.ts b/packages/next/src/server/dynamic-rendering-utils.test.ts new file mode 100644 index 000000000000..46c961faae2c --- /dev/null +++ b/packages/next/src/server/dynamic-rendering-utils.test.ts @@ -0,0 +1,114 @@ +import { createPromiseWithResolvers } from '../shared/lib/promise-with-resolvers' +import { trackPromiseUsed } from './dynamic-rendering-utils' + +describe('trackPromiseUsed', () => { + it('`then` is tracked and forwards the value', async () => { + const underlying = createPromiseWithResolvers() + + const onUse = jest.fn() + + const trackedPromise = trackPromiseUsed(underlying.promise, onUse) + void trackedPromise.then // accessing the property does not count as a usage + expect(onUse).toHaveBeenCalledTimes(0) + + const derived = trackedPromise.then((value) => { + return value + }) + expect(onUse).toHaveBeenCalledTimes(1) + expect(derived).toBeInstanceOf(Promise) + + const result = { foo: 'bar' } + underlying.resolve(result) + + await expect(derived).resolves.toBe(result) + }) + + it('`catch` is tracked and forwards the error', async () => { + const underlying = createPromiseWithResolvers() + + const onUse = jest.fn() + + const trackedPromise = trackPromiseUsed(underlying.promise, onUse) + void trackedPromise.catch // accessing the property does not count as a usage + expect(onUse).toHaveBeenCalledTimes(0) + + const derived = trackedPromise.catch((error) => { + return error + }) + expect(onUse).toHaveBeenCalledTimes(1) + expect(derived).toBeInstanceOf(Promise) + + const error = new Error('kaboom') + underlying.reject(error) + + // Note: we're catching the error, so it resolves, not rejects + await expect(derived).resolves.toBe(error) + }) + + it('`finally` is tracked and forwards the value', async () => { + const underlying = createPromiseWithResolvers() + + const onUse = jest.fn() + + const trackedPromise = trackPromiseUsed(underlying.promise, onUse) + void trackedPromise.finally // accessing the property does not count as a usage + expect(onUse).toHaveBeenCalledTimes(0) + + const onFinally = jest.fn() + const derived = trackedPromise.finally(onFinally) + expect(onUse).toHaveBeenCalledTimes(1) + expect(derived).toBeInstanceOf(Promise) + + const result = { foo: 'bar' } + underlying.resolve(result) + + await expect(derived).resolves.toBe(result) + expect(onFinally).toHaveBeenCalledTimes(1) + }) + + it('`finally` is tracked and forwards the error', async () => { + const underlying = createPromiseWithResolvers() + + const onUse = jest.fn() + + const trackedPromise = trackPromiseUsed(underlying.promise, onUse) + void trackedPromise.finally // accessing the property does not count as a usage + expect(onUse).toHaveBeenCalledTimes(0) + + const onFinally = jest.fn() + const derived = trackedPromise.finally(onFinally) + expect(onUse).toHaveBeenCalledTimes(1) + expect(derived).toBeInstanceOf(Promise) + + const error = new Error('kaboom') + underlying.reject(error) + + await expect(derived).rejects.toBe(error) + }) + + it('native `await` is tracked', async () => { + const underlying = createPromiseWithResolvers() + + const onUse = jest.fn() + + const trackedPromise = trackPromiseUsed(underlying.promise, onUse) + expect(onUse).toHaveBeenCalledTimes(0) + + const derived = (async () => { + const result = await trackedPromise + return result + })() + + // Flush microtasks + await new Promise((resolve) => + queueMicrotask(() => process.nextTick(resolve)) + ) + + expect(onUse).toHaveBeenCalledTimes(1) + + const result = { foo: 'bar' } + underlying.resolve(result) + + await expect(derived).resolves.toBe(result) + }) +}) diff --git a/packages/next/src/server/dynamic-rendering-utils.ts b/packages/next/src/server/dynamic-rendering-utils.ts index 33b30b78975d..5b2330507832 100644 --- a/packages/next/src/server/dynamic-rendering-utils.ts +++ b/packages/next/src/server/dynamic-rendering-utils.ts @@ -2,13 +2,13 @@ import { RenderStage, type AdvanceableRenderStage, } from './app-render/staged-rendering' +import { workAsyncStorage } from './app-render/work-async-storage.external' import type { RequestStore, WorkUnitStore, } from './app-render/work-unit-async-storage.external' import { workUnitAsyncStorage } from './app-render/work-unit-async-storage.external' import { getServerReact, getClientReact } from './runtime-reacts.external' -import { ReflectAdapter } from './web/spec-extension/adapters/reflect' export function isHangingPromiseRejectionError( err: unknown @@ -112,7 +112,7 @@ export function makeUntrackedHangingPromise( * searchParams, and cache entries that are excluded only from static * prerenders. * - * Creating one of these during a static prerender records on the prerender + * Awaiting one of these during a static prerender records on the prerender * store that a runtime prefetch would produce more content than the static * response (`runtimeDataAccessed`), which the segment prefetch encoding uses * to tell the client whether a runtime prefetch request could be skipped. @@ -121,13 +121,10 @@ export function makeUntrackedHangingPromise( * cost of over-recording is a redundant runtime prefetch request; the cost of * under-recording is a permanently missing one. * - * `workUnitStore` may be null ONLY when the caller tracks the access itself - * at observation time instead of creation time. This is for promises the - * framework creates eagerly whether or not anything reads them (e.g. the - * `searchParams` prop constructed for every page): recording at creation - * would mark every render. Such a caller MUST call `trackRuntimeDataAccessed` - * from every path that observes the promise (e.g. the proxy traps for - * `then`/`status`), against the work unit store active at access time. + * `workUnitStore` may be null ONLY when the caller tracks the access itself. + * Such a caller MUST call `trackRuntimeDataAccessed` from every path that + * observes the promise (e.g. the proxy traps for `then`/`status`), + * against the work unit store active at access time. * * For fallback-param data — data a concrete (ISR-upgraded) prerender would * resolve — use `makeFallbackParamsHangingPromise` instead, so the access @@ -141,29 +138,31 @@ export function makeRuntimeHangingPromise( expression: string, workUnitStore: WorkUnitStore | null ): Promise { - if (workUnitStore !== null) { - trackRuntimeDataAccessed(workUnitStore) - } - return makeHangingPromiseWithError( + const promise = makeHangingPromiseWithError( signal, new HangingPromiseRejectionError(route, expression) ) + if (workUnitStore === null) { + return promise + } + return trackPromiseUsed( + promise, + trackRuntimeDataAccessed.bind(null, workUnitStore, expression) + ) } /** * Variant of `makeRuntimeHangingPromise` for *fallback-param* data: fallback * route params and values derived solely from them (`params`, `rootParams`, - * `pathname` during a fallback prerender). Like every runtime data access it - * records the access on the prerender store's response-level flag, but its - * effect on the build-time static-prefetch hint differs — on a + * `pathname` during a fallback prerender). Like every runtime data access, + * awaiting it records the access on the prerender store's response-level flag, + * but its effect on the build-time static-prefetch hint differs — on a * fallback-upgradeable route the access is transient (a concrete prerender - * resolves it), so it leaves the hint intact. See - * `trackFallbackParamsAccessed`. + * resolves it), so it leaves the hint intact. See `trackFallbackParamsAccessed`. * * As with `makeRuntimeHangingPromise`, `workUnitStore` may be null ONLY when - * the caller tracks the access itself at observation time instead of creation - * time, by calling `trackFallbackParamsAccessed` from every path that - * observes the promise. + * the caller tracks the access itself by calling `trackFallbackParamsAccessed` + * from every path that observes the promise. * * @internal */ @@ -173,13 +172,17 @@ export function makeFallbackParamsHangingPromise( expression: string, workUnitStore: WorkUnitStore | null ): Promise { - if (workUnitStore !== null) { - trackFallbackParamsAccessed(workUnitStore) - } - return makeHangingPromiseWithError( + const promise = makeHangingPromiseWithError( signal, new HangingPromiseRejectionError(route, expression) ) + if (workUnitStore === null) { + return promise + } + return trackPromiseUsed( + promise, + trackFallbackParamsAccessed.bind(null, workUnitStore, expression) + ) } /** @@ -191,8 +194,8 @@ export function makeFallbackParamsHangingPromise( * * A render that runs through the later stage would include the data; in * particular a runtime prefetch renders through its later stages, so on a - * static prerender store this records `runtimeDataAccessed`, same as - * `makeRuntimeHangingPromise`. + * static prerender store awaiting this promise records `runtimeDataAccessed`, + * same as `makeRuntimeHangingPromise`. * * @internal */ @@ -202,10 +205,12 @@ export function makeStageHangingPromise( expression: string, workUnitStore: WorkUnitStore ): Promise { - trackRuntimeDataAccessed(workUnitStore) - return makeHangingPromiseWithError( - signal, - new HangingPromiseRejectionError(route, expression) + return trackPromiseUsed( + makeHangingPromiseWithError( + signal, + new HangingPromiseRejectionError(route, expression) + ), + trackRuntimeDataAccessed.bind(null, workUnitStore, expression) ) } @@ -214,18 +219,18 @@ export function makeStageHangingPromise( * which would have resolved during a runtime prerender. No-op for all other * store types. * - * `makeRuntimeHangingPromise` and `makeStageHangingPromise` call this - * automatically; call it directly only where the access is observed - * separately from the promise's creation (see the null `workUnitStore` case - * of `makeRuntimeHangingPromise`), or where the prerender is aborted - * synchronously instead of hanging. + * Prefer `makeRuntimeHangingPromise` and `makeStageHangingPromise`. + * Use this method only when implementing similar tracking and those two are not enough. * * For fallback-param data, use `trackFallbackParamsAccessed` instead. When * unsure, this is the conservative choice: it unconditionally clears the * static-prefetch hint. */ -export function trackRuntimeDataAccessed(workUnitStore: WorkUnitStore): void { - trackRuntimeDataAccessedImpl(workUnitStore, false) +export function trackRuntimeDataAccessed( + workUnitStore: WorkUnitStore, + expression: string +): void { + trackRuntimeDataAccessedImpl(workUnitStore, false, expression) } /** @@ -237,14 +242,16 @@ export function trackRuntimeDataAccessed(workUnitStore: WorkUnitStore): void { * concrete prerender that resolves it. */ export function trackFallbackParamsAccessed( - workUnitStore: WorkUnitStore + workUnitStore: WorkUnitStore, + expression: string ): void { - trackRuntimeDataAccessedImpl(workUnitStore, true) + trackRuntimeDataAccessedImpl(workUnitStore, true, expression) } function trackRuntimeDataAccessedImpl( workUnitStore: WorkUnitStore, - isFallbackParamAccess: boolean + isFallbackParamAccess: boolean, + expression: string ): void { switch (workUnitStore.type) { case 'prerender': { @@ -283,6 +290,13 @@ function trackRuntimeDataAccessedImpl( hintCell !== null && (!isFallbackParamAccess || !workUnitStore.isFallbackUpgradeable) ) { + if (process.env.NEXT_PRIVATE_DEBUG_RUNTIME_DATA) { + const workStore = workAsyncStorage.getStore() + const route = workStore?.route ?? '' + console.log( + `Route '${route}' deopting to runtime requests because it used ${expression}` + ) + } hintCell.current = false } break @@ -389,38 +403,72 @@ export function makeDevtoolsIOAwarePromise( }) } -/** Invokes `onUse` whenever `then()/catch()/finally()` are called on the promise. */ -export function trackPromiseUsed(promise: Promise, onUse: () => void) { - const methodCache: Record any> = {} - return new Proxy(promise, { - get(target, prop, receiver) { - if (prop === 'then' || prop === 'catch' || prop === 'finally') { - let patchedMethod = methodCache[prop] - if (patchedMethod !== undefined) { - return patchedMethod - } +/** + * Invokes `onUse` whenever `then()/catch()/finally()` are called on the promise + * or when the promise is awaited. */ +export function trackPromiseUsed( + promise: Promise, + onUse: () => void +): Promise { + // We can instrument `.then()/.catch()/.finally()` in one go by using a Promise subclass + // that implements a custom `.then()`, because `catch` and `finally` delegate to it. + // + // Alternative implementation ideas that were tried and rejected: + // + // 1. Patching the methods directly via `promise.then = (..args) => { ... }`: + // doesn't work, because Node does not call the monkeypatched methods for native `await`: + // > Native Promise [...]: The promise is directly used and awaited natively, without calling `then()`. + // > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await#description + // + // 2. Wrapping in a proxy that returns a custom `then/catch/finally`: + // breaks async stacks in React's IO tracking (stack becomes `Promise.then`) + return TrackedPromise.from(promise, onUse) +} - const originalMethod = ReflectAdapter.get(target, prop, receiver) - patchedMethod = { - [prop]: (...args: unknown[]) => { - try { - onUse() - } catch (err) { - // We don't want to break the method even if our tracking errored. - console.error(err) - } +class TrackedPromise extends Promise { + #onUse: (() => void) | null = null - return originalMethod.apply(target, args) - }, - }[prop] + // We don't need derived promises to also be a TrackedPromise. + // We only care about the first level of `.then()`. + static get [Symbol.species]() { + return Promise + } - methodCache[prop] = patchedMethod - return patchedMethod + static from(promise: Promise, onUse: () => void): TrackedPromise { + // Whenever the promise we're tracking resolves/rejects, we should follow. + const tracked = new TrackedPromise(promise.then.bind(promise)) + + tracked.#onUse = onUse + + // Hanging promises catch rejections when created. Tracked promises are generally derived + // from promises that may hang & reject, so we need to do the same. + // However, we have to bypass the tracking we do in `TrackedPromise.then`. + // (we're using `then` directly, because `catch` ends up delegating `TrackedPromise.then`) + Promise.prototype.then.call(tracked, undefined, ignoreReject) + + return tracked + } + + then( + onFulfilled?: (value: T) => TResult1 | PromiseLike, + onRejected?: (reason: unknown) => TResult2 | PromiseLike + ): Promise { + const onUse = this.#onUse + if (onUse) { + try { + onUse() + } catch (err) { + // We don't want to break the method even if our tracking errored. + console.error(err) } + } - return ReflectAdapter.get(target, prop, receiver) - }, - }) + return Promise.prototype.then.call( + this, + onFulfilled, + onRejected + ) as Promise + } } export const RENDER_STAGES_BY_DATA_KIND = { diff --git a/packages/next/src/server/request/params.ts b/packages/next/src/server/request/params.ts index 8d2a48381c0a..9e01ada7c870 100644 --- a/packages/next/src/server/request/params.ts +++ b/packages/next/src/server/request/params.ts @@ -686,7 +686,7 @@ const fallbackParamsProxyHandler: ProxyHandler> = { // rendering when it's finally awaited. const workUnitStore = workUnitAsyncStorage.getStore() if (workUnitStore !== undefined) { - trackFallbackParamsAccessed(workUnitStore) + trackFallbackParamsAccessed(workUnitStore, '`params`') } const store = dynamicAccessAsyncStorage.getStore() @@ -724,9 +724,9 @@ function makeHangingParams( prerenderStore.renderSignal, workStore.route, '`params`', - // This promise is created for every segment on a fallback route whether - // or not it reads params, so recording the access at creation would mark - // every render. The access is tracked in the proxy traps instead. + // Passing `null` for the store disables tracking of params usage. + // Caches need the additional logic from `fallbackParamsProxyHandler`, + // so we track params usage there instead. null ), fallbackParamsProxyHandler diff --git a/packages/next/src/server/request/search-params.ts b/packages/next/src/server/request/search-params.ts index 495e2655d854..3f0760fdae4c 100644 --- a/packages/next/src/server/request/search-params.ts +++ b/packages/next/src/server/request/search-params.ts @@ -393,7 +393,7 @@ function makeHangingSearchParams( // created while the RSC payload is constructed, but typically accessed // later, during the render, under a different store. const workUnitStore = workUnitAsyncStorage.getStore() - trackRuntimeDataAccessed(workUnitStore ?? prerenderStore) + trackRuntimeDataAccessed(workUnitStore ?? prerenderStore, '`searchParams`') } const proxyHandler: ProxyHandler> = { diff --git a/scripts/run-jest.sh b/scripts/run-jest.sh index 59032799761a..c1bbedb06adf 100755 --- a/scripts/run-jest.sh +++ b/scripts/run-jest.sh @@ -56,6 +56,35 @@ while [ $# -gt 0 ]; do shift done +# `__NEXT_TEST_AXIS` names the alternate flag configurations of the test +# matrix. Axes are lettered (`A`, `B`, …) — a fixed enumeration a fixture opts +# into, not a boolean, and not one of the buckets test *sharding* splits a run +# into. CI runs the suites once plainly and once per axis, and a fixture keys +# an experimental flag on an axis to cover both states of the flag — enabled +# by default, disabled on that axis: +# +# experimental: { +# concurrentRouterQueue: process.env.__NEXT_TEST_AXIS !== 'A', +# } +# +# paired with a `// @gate concurrentRouterQueue` on the affected tests: a +# plain run — including a local run with no special env — exercises the +# feature, and the axis run covers the off state (see +# test/lib/gate/README.md). +# +# For now there is a single axis, `A`, and it is an alias for +# `__NEXT_CACHE_COMPONENTS` (the `--experimental` run) rather than a CI +# dimension of its own. That works because most experiments hard-code +# `cacheComponents: true` in their fixture anyway — the cache-components env +# default only applies to fixtures that don't set it themselves, so for these +# fixtures that run is free to double as the axis run. Setting either name +# implies the other. +if [ -n "${__NEXT_TEST_AXIS:-}" ]; then + export __NEXT_CACHE_COMPONENTS=true +elif [ "${__NEXT_CACHE_COMPONENTS:-}" = "true" ]; then + export __NEXT_TEST_AXIS=A +fi + # Resolves to `node_modules/.bin/jest` via `$PATH`. This relies on being # invoked through pnpm (or another package runner), which prepends the # workspace's `node_modules/.bin/` to `$PATH` before running the script. diff --git a/test/e2e/app-dir/cache-components-request-apis/cache-components-request-apis.test.ts b/test/e2e/app-dir/cache-components-request-apis/cache-components-request-apis.test.ts index d95fd4132d58..8aadef5c474a 100644 --- a/test/e2e/app-dir/cache-components-request-apis/cache-components-request-apis.test.ts +++ b/test/e2e/app-dir/cache-components-request-apis/cache-components-request-apis.test.ts @@ -60,11 +60,15 @@ describe(`Request Promises`, () => { } const expectError = createExpectError(next.cliOutput) + expectError( + 'Error: During prerendering, `searchParams` rejects when the prerender is complete' + ) expectError( 'Error: During prerendering, `params` rejects when the prerender is complete' ) + expectError( - 'Error: During prerendering, `searchParams` rejects when the prerender is complete' + 'Error: During prerendering, `connection()` rejects when the prerender is complete' ) expectError( 'Error: During prerendering, `cookies()` rejects when the prerender is complete' @@ -72,9 +76,6 @@ describe(`Request Promises`, () => { expectError( 'Error: During prerendering, `headers()` rejects when the prerender is complete' ) - expectError( - 'Error: During prerendering, `connection()` rejects when the prerender is complete' - ) }) }) describe('On Prerender Interruption', () => { @@ -100,11 +101,12 @@ describe(`Request Promises`, () => { const expectError = createExpectError(next.cliOutput) expectError( - 'Error: During prerendering, `params` rejects when the prerender is complete' + 'Error: During prerendering, `searchParams` rejects when the prerender is complete' ) expectError( - 'Error: During prerendering, `searchParams` rejects when the prerender is complete' + 'Error: During prerendering, `params` rejects when the prerender is complete' ) + expectError( 'Error: During prerendering, `cookies()` rejects when the prerender is complete' ) diff --git a/test/e2e/app-dir/concurrent-router-queue/concurrent-router-queue.test.ts b/test/e2e/app-dir/concurrent-router-queue/concurrent-router-queue.test.ts index 2320b8b17b5e..554ef15f090b 100644 --- a/test/e2e/app-dir/concurrent-router-queue/concurrent-router-queue.test.ts +++ b/test/e2e/app-dir/concurrent-router-queue/concurrent-router-queue.test.ts @@ -17,6 +17,7 @@ describe('concurrent-router-queue', () => { files: __dirname, }) + // Not gated: a clean hydration is expected in both states of the flag. it('hydrates cleanly without invoking the forked entry points', async () => { // `pushErrorAsConsoleLog` records uncaught page errors into the console // log capture, which works in both dev and start modes. @@ -29,6 +30,10 @@ describe('concurrent-router-queue', () => { expect(errors).toEqual([]) }) + // The stubs only throw when the fork is active; with the flag off, the + // sequential router handles the navigation and the test fails its + // expectations — which is what the gate asserts on the axis-A run. + // @gate concurrentRouterQueue it('fails loudly on link navigation', async () => { const browser = await next.browser('/', { pushErrorAsConsoleLog: true }) await browser.waitForElementByCss('#invoke-action') @@ -55,6 +60,7 @@ describe('concurrent-router-queue', () => { expect(await browser.hasElementByCssSelector('#target-page')).toBe(false) }) + // @gate concurrentRouterQueue it('fails loudly on server action invocation', async () => { const browser = await next.browser('/') await browser.waitForElementByCss('#invoke-action') diff --git a/test/e2e/app-dir/concurrent-router-queue/next.config.js b/test/e2e/app-dir/concurrent-router-queue/next.config.js index 1f8fd1dd240d..fe58dfa8eef5 100644 --- a/test/e2e/app-dir/concurrent-router-queue/next.config.js +++ b/test/e2e/app-dir/concurrent-router-queue/next.config.js @@ -2,8 +2,15 @@ * @type {import('next').NextConfig} */ const nextConfig = { + // Pin every dimension except the one under test. + cacheComponents: true, experimental: { - concurrentRouterQueue: true, + // Keyed on test axis A (see scripts/run-jest.sh) so the suite covers + // both states. Enabled by default — a plain run exercises the fork with + // no special env — and disabled on axis A, where the + // `@gate concurrentRouterQueue` tests assert the sequential router is + // back in charge. + concurrentRouterQueue: process.env.__NEXT_TEST_AXIS !== 'A', }, } diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/page.tsx b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/page.tsx index d44acbfd8dac..1001d5b21908 100644 --- a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/page.tsx +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/page.tsx @@ -1,3 +1,4 @@ +import Link from 'next/link' import { LinkAccordion } from '../components/link-accordion' export default function Page() { @@ -17,6 +18,56 @@ export default function Page() {
  • Uses cookies
    • +
  • + + Calls Runtime APIs but does not await them + +
    • +
  • + Params after dynamic content +
      +
    • + + Param 1 + +
      • +
    • + + Param 2 (unprefetched) + +
      • +
    +
    • +
  • + Params after navigation() +
      +
    • + + Param 1 + +
      • +
    • + + Param 2 (unprefetched) + +
      • +
    +
    • +
  • + Params used in icon +
      +
    • + + Param 1 + +
      • +
    • + + Params 2 + +
      • +
    +
  • Uses search params diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-after-dynamic/[id]/page.tsx b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-after-dynamic/[id]/page.tsx new file mode 100644 index 000000000000..c00ac1203cc5 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-after-dynamic/[id]/page.tsx @@ -0,0 +1,34 @@ +import { connection } from 'next/server' +import { Suspense } from 'react' + +type Props = { params: Promise<{ id: string }> } + +export default async function Page(props: Props) { + return ( +
    +

    Params awaited after dynamic data

    + Loading dynamic content...

    } + > + +     +
    + ) +} + +async function Dynamic(props: Props) { + // The prerender ends here, so it doesn't observe params being awaited. + await connection() + + return ( + <> +
    Dynamic content
    + + + ) +} + +async function ParamsDependent(props: Props) { + const { id } = await props.params + return

    Post: {id}

    +} diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-after-navigation/[id]/page.tsx b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-after-navigation/[id]/page.tsx new file mode 100644 index 000000000000..3316bc32ed81 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-after-navigation/[id]/page.tsx @@ -0,0 +1,36 @@ +import { unstable_navigation as navigation } from 'next/cache' +import { Suspense } from 'react' + +type Props = { params: Promise<{ id: string }> } + +export default async function Page(props: Props) { + return ( +
    +

    Params awaited after navigation

    + Loading navigation content...

    } + > + +     +
    + ) +} + +async function NavigationOnly(props: Props) { + // navigation() does not resolve in runtime prefetches, so awaiting `params` + // after `navigation` should not deopt this page to using runtime requests + // (because runtime shells/prefetches would not provide more data) + await navigation() + + return ( + <> +
    Navigation content
    + + + ) +} + +async function ParamsDependent(props: Props) { + const { id } = await props.params + return

    Post: {id}

    +} diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-in-icon/[id]/icon.tsx b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-in-icon/[id]/icon.tsx new file mode 100644 index 000000000000..c22a8f927330 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-in-icon/[id]/icon.tsx @@ -0,0 +1,23 @@ +import { ImageResponse } from 'next/og' + +export default async function icon({ params }) { + const { id } = await params + return new ImageResponse( + ( +
    + P{id} +
    + ) + ) +} diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-in-icon/[id]/page.tsx b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-in-icon/[id]/page.tsx new file mode 100644 index 000000000000..a932dfeea9bf --- /dev/null +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/params-used-in-icon/[id]/page.tsx @@ -0,0 +1,34 @@ +import { connection } from 'next/server' +import { Suspense } from 'react' + +type Props = { params: Promise<{ id: string }> } + +export default async function Page(props: Props) { + return ( +
    +

    Params awaited in icon.tsx and after dynamic data

    + Loading dynamic content...

    } + > + +     +
    + ) +} + +async function Dynamic(props: Props) { + // The prerender ends here, so it doesn't observe params being awaited. + await connection() + + return ( + <> +
    Dynamic content
    + + + ) +} + +async function ParamsDependent(props: Props) { + const { id } = await props.params + return

    Post: {id}

    +} diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/runtime-called-but-not-awaited/page.tsx b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/runtime-called-but-not-awaited/page.tsx new file mode 100644 index 000000000000..a3c8216297a0 --- /dev/null +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/app/runtime-called-but-not-awaited/page.tsx @@ -0,0 +1,43 @@ +// A page that calls cookies and headers, but doesn't await them during the prerender, +// which means it can still be prefetched statically. +// +// Note the Shell phase is always permitted to issue a runtime shell request, +// so the absence of one in the test is attributable to the static shell +// attempt succeeding, not to configuration forbidding runtime requests. + +import { cacheLife } from 'next/dist/server/use-cache/cache-life' +import { cookies, headers } from 'next/headers' +import { connection } from 'next/server' +import { Suspense } from 'react' + +export default async function Page() { + return ( +
    +

    Runtime APIs called but not awaited

    + Loading dynamic content...

    } + > + +     +
    + ) +} + +async function Dynamic() { + const cookiesPromise = cookies() + const headersPromise = headers() + const cachePromise = shortStaleCache() + + // The prerender ends here, so it doesn't observe cookies/headers being awaited. + await connection() + + await Promise.all([cookiesPromise, headersPromise, cachePromise]) + + return
    Dynamic content
    +} + +async function shortStaleCache() { + 'use cache' + cacheLife({ stale: 300 - 1 }) // smaller than MIN_SHELL_STALE + return Date.now() +} diff --git a/test/e2e/app-dir/segment-cache/prefetch-static-shell/prefetch-static-shell.test.ts b/test/e2e/app-dir/segment-cache/prefetch-static-shell/prefetch-static-shell.test.ts index b95df1163cf6..ba6fb7a73877 100644 --- a/test/e2e/app-dir/segment-cache/prefetch-static-shell/prefetch-static-shell.test.ts +++ b/test/e2e/app-dir/segment-cache/prefetch-static-shell/prefetch-static-shell.test.ts @@ -254,6 +254,249 @@ describe('static App Shell prefetch attempt', () => { ]) }) + it("uses a static app shell for a partial segment that calls runtime APIs but doesn't await them", async () => { + let page: Playwright.Page + const browser = await next.browser('/', { + beforePageLoad(p: Playwright.Page) { + page = p + }, + }) + const act = createRouterAct(page, { includeAppShellRequests: true }) + + // Reveal the LinkAccordion for /runtime-called-but-not-awaited. + // No runtime data was awaited, so a static app shell is sufficient + // (a runtime app shell would not provide more data) + await act(async () => { + await browser + .elementByCss( + 'input[data-link-accordion="/runtime-called-but-not-awaited"]' + ) + .click() + }, [ + { includes: 'Runtime APIs called but not awaited', kind: 'static' }, + // We only expect a static prefetch. + { + includes: 'Runtime APIs called but not awaited', + kind: 'runtime', + block: 'reject', + }, + { includes: 'Dynamic content', kind: 'runtime', block: 'reject' }, + ]) + + // Navigate. The prefetched shell renders instantly, and the dynamic data arrives + // later, as part of the navigation request. + await act( + async () => { + await browser + .elementByCss('a[href="/runtime-called-but-not-awaited"]') + .click() + + // While the navigation response is blocked (we're still inside the + // `act` scope), the prefetched shell is already visible, with the + // loading fallback in place of the dynamic content. + expect(await browser.elementById('page-content').text()).toBe( + 'Runtime APIs called but not awaited' + ) + expect(await browser.elementById('dynamic-loading').text()).toBe( + 'Loading dynamic content...' + ) + }, + // The dynamic content streams in with the navigation response. + { includes: 'Dynamic content' } + ) + + expect(await browser.elementById('dynamic-content').text()).toBe( + 'Dynamic content' + ) + }) + + it('uses a static app shell for a partial segment that only awaits params after dynamic data', async () => { + let page: Playwright.Page + const browser = await next.browser('/', { + beforePageLoad(p: Playwright.Page) { + page = p + }, + }) + const act = createRouterAct(page, { includeAppShellRequests: true }) + + // Reveal the LinkAccordion for /params-used-after-dynamic/1. + // No runtime data was awaited, so a static app shell is sufficient + // (a runtime app shell would not provide more data) + await act(async () => { + await browser + .elementByCss( + 'input[data-link-accordion="/params-used-after-dynamic/1"]' + ) + .click() + }, [ + { includes: 'Params awaited after dynamic data', kind: 'static' }, + // We only expect a static prefetch, no runtime requests. + { + includes: 'Params awaited after dynamic data', + kind: 'runtime', + block: 'reject', + }, + { includes: 'Dynamic content', kind: 'runtime', block: 'reject' }, + ]) + + // Navigate to an unprefetched link with a different param value. + // This should re-use the app shell that we got when we prefetched /1. + await act( + async () => { + await browser + .elementByCss('a[href="/params-used-after-dynamic/2"]') + .click() + + // While the navigation response is blocked (we're still inside the + // `act` scope), the prefetched shell is already visible, with the + // loading fallback in place of the dynamic content. + expect(await browser.elementById('page-content').text()).toBe( + 'Params awaited after dynamic data' + ) + expect(await browser.elementById('dynamic-loading').text()).toBe( + 'Loading dynamic content...' + ) + }, + // The dynamic content streams in with the navigation response. + { includes: 'Dynamic content' } + ) + + expect(await browser.elementById('dynamic-content').text()).toBe( + 'Dynamic content' + ) + expect(await browser.elementById('param-value').text()).toBe('Post: 2') + }) + + it('uses a static app shell for a partial segment that only awaits params after navigation()', async () => { + let page: Playwright.Page + const browser = await next.browser('/', { + beforePageLoad(p: Playwright.Page) { + page = p + }, + }) + const act = createRouterAct(page, { includeAppShellRequests: true }) + + // Reveal the LinkAccordion for /params-used-after-navigation/1. + // No runtime data was awaited, so a static app shell is sufficient + // (a runtime app shell would not provide more data) + await act(async () => { + await browser + .elementByCss( + 'input[data-link-accordion="/params-used-after-navigation/1"]' + ) + .click() + }, [ + { includes: 'Params awaited after navigation', kind: 'static' }, + // We only expect a static prefetch, no runtime requests. + { + includes: 'Params awaited after navigation', + kind: 'runtime', + block: 'reject', + }, + ]) + + // Navigate to an unprefetched link with a different param value. + // This should re-use the app shell that we got when we prefetched /1. + await act( + async () => { + await browser + .elementByCss('a[href="/params-used-after-navigation/2"]') + .click() + + expect(await browser.elementById('page-content').text()).toBe( + 'Params awaited after navigation' + ) + + // `navigation()` *does* resolve in static prefetches so we have navigation-gated + // content for /1. However, it is not considered part of the app shell, so it should + // not be visible here. + expect(await browser.elementById('navigation-loading').text()).toBe( + 'Loading navigation content...' + ) + }, + // The navigation content streams in with the navigation response. + { includes: 'Navigation content' } + ) + + expect(await browser.elementById('navigation-content').text()).toBe( + 'Navigation content' + ) + expect(await browser.elementById('param-value').text()).toBe('Post: 2') + }) + + it('uses a runtime shell for a partial segment that has a param-dependent icon.tsx', async () => { + let page: Playwright.Page + const browser = await next.browser('/', { + beforePageLoad(p: Playwright.Page) { + page = p + }, + }) + const act = createRouterAct(page, { includeAppShellRequests: true }) + + // Reveal the LinkAccordion for /params-used-in-icon/1. + // No runtime data was awaited in the page itself during the prerender, + // but the head is param-dependent because it needs to link to the + // param-dependent icon: + // + // + // + // which is tracked as a runtime data access and deopts the page + // to runtime requests. + await act(async () => { + await browser + .elementByCss('input[data-link-accordion="/params-used-in-icon/1"]') + .click() + }, [ + { + includes: 'Params awaited in icon.tsx and after dynamic data', + kind: 'runtime', + }, + { + includes: 'Params awaited in icon.tsx and after dynamic data', + kind: 'static', + block: 'reject', + }, + { includes: 'Dynamic content', kind: 'static', block: 'reject' }, + ]) + + // Navigate to an unprefetched link with a different param value. + // This should re-use the app shell that we got when we prefetched /1. + await act( + async () => { + await browser.elementByCss('a[href="/params-used-in-icon/2"]').click() + + // While the navigation response is blocked (we're still inside the + // `act` scope), the prefetched shell is already visible, with the + // loading fallback in place of the dynamic content. + expect(await browser.elementById('page-content').text()).toBe( + 'Params awaited in icon.tsx and after dynamic data' + ) + + // The icon is param-dependent and should not be part of the shell. + expect(await browser.locator('link[rel="icon"]').count()).toBe(0) + + expect(await browser.elementById('dynamic-loading').text()).toBe( + 'Loading dynamic content...' + ) + }, + // The dynamic content streams in with the navigation response. + { includes: 'Dynamic content' } + ) + + expect(await browser.elementById('dynamic-content').text()).toBe( + 'Dynamic content' + ) + + expect( + new URL( + await browser.elementByCss('link[rel="icon"]').getAttribute('href'), + 'http://__n' + ).pathname + ).toEqual('/params-used-in-icon/2/icon') + + expect(await browser.elementById('param-value').text()).toBe('Post: 2') + }) + it('does not fall back to a runtime shell prefetch for a partial segment whose holes are dynamic (connection)', async () => { let page: Playwright.Page const browser = await next.browser('/', { diff --git a/test/e2e/app-dir/use-offline/next.config.js b/test/e2e/app-dir/use-offline/next.config.js index 420686102585..440e5b14cd31 100644 --- a/test/e2e/app-dir/use-offline/next.config.js +++ b/test/e2e/app-dir/use-offline/next.config.js @@ -4,7 +4,14 @@ const nextConfig = { cacheComponents: true, experimental: { - useOffline: true, + // Keyed on test axis A (see scripts/run-jest.sh). Enabled by default — + // a plain run exercises the hook with no special env — and disabled on + // axis A, where the `@force-gate useOffline` on the suite skips the + // fixture build entirely: `useOffline()` is a new API that is inert when + // disabled (it always reports online), so the off state has nothing to + // assert and would only fail by timing out. The keying turns a redundant + // duplicate run into a near-free skip. + useOffline: process.env.__NEXT_TEST_AXIS !== 'A', varyParams: true, optimisticRouting: true, cachedNavigations: true, diff --git a/test/e2e/app-dir/use-offline/use-offline.test.ts b/test/e2e/app-dir/use-offline/use-offline.test.ts index 86d844e8de7c..812eb9a6ab4f 100644 --- a/test/e2e/app-dir/use-offline/use-offline.test.ts +++ b/test/e2e/app-dir/use-offline/use-offline.test.ts @@ -3,16 +3,13 @@ import type * as Playwright from 'playwright' import { createRouterAct } from 'router-act' import { retry } from 'next-test-utils' +// @force-gate prefetching +// @force-gate useOffline describe('useOffline', () => { - const { next, isNextDev } = nextTestSetup({ + const { next } = nextTestSetup({ files: __dirname, }) - if (isNextDev) { - test('skipped in dev mode', () => {}) - return - } - // Uses Playwright's built-in network emulation, which fires the browser's // native offline/online events and blocks all requests at the network layer. async function goOffline(page: Playwright.Page) { diff --git a/test/jest-setup-after-env.ts b/test/jest-setup-after-env.ts index dcf3748a9d28..b4f11b2db3ba 100644 --- a/test/jest-setup-after-env.ts +++ b/test/jest-setup-after-env.ts @@ -1,6 +1,13 @@ import * as matchers from 'jest-extended' +import { installGate } from './lib/gate/runtime' + expect.extend(matchers) +// Installs the `_test_gate` global that `// @gate` pragmas compile to, and +// wraps `it`/`test` so a gate on a `describe` reaches the tests inside it. +// See test/lib/gate/runtime.ts. +installGate() + // Patch jscodeshift testUtils to normalize line endings (fixes Windows CRLF issues) // The issue: jscodeshift's printer (recast) outputs CRLF on Windows, but test fixtures use LF // We need to patch both defineTest (which uses internal closure references) and runInlineTest diff --git a/test/lib/e2e-utils/index.ts b/test/lib/e2e-utils/index.ts index 475f5324655b..ed5f08e8917f 100644 --- a/test/lib/e2e-utils/index.ts +++ b/test/lib/e2e-utils/index.ts @@ -1,12 +1,23 @@ import path from 'path' import assert from 'assert' import { flushAllTraces, setGlobal, trace } from 'next/dist/trace' -import { PHASE_DEVELOPMENT_SERVER } from 'next/constants' +import { + PHASE_DEVELOPMENT_SERVER, + PHASE_PRODUCTION_BUILD, +} from 'next/constants' import { NextInstance, NextInstanceOpts } from '../next-modes/base' import { NextDevInstance } from '../next-modes/next-dev' import { NextStartInstance } from '../next-modes/next-start' import { NextDeployInstance } from '../next-modes/next-deploy' import { shouldUseTurbopack } from '../next-test-utils' +import { setGateTestContext, type GateTestMode } from '../gate/test-context' +import { clearFixture, registerFixture } from '../gate/state' +import { loadResolvedConfig } from '../gate/load-resolved-config' +import { + getActiveDescribeGates, + hasLazyForceGate, + findLazyForceSkip, +} from '../gate/runtime' export type { NextInstance } export type { Playwright } from '../browsers/playwright' @@ -177,6 +188,19 @@ export const itTurbopack = export const isReact18 = parseInt(process.env.NEXT_TEST_REACT_VERSION || '', 10) === 18 +// Publish the statically-known shape of this run for `// @gate` pragmas. See +// test/lib/gate/conditions.ts. +setGateTestContext({ + mode: testMode as GateTestMode, + bundler: isRspack + ? 'rspack' + : !isNextTestWasm && shouldUseTurbopack() + ? 'turbopack' + : 'webpack', + react18: isReact18, + wasm: isNextTestWasm, +}) + if (!testMode) { throw new Error( `No 'NEXT_TEST_MODE' set in environment, this is required for e2e-utils` @@ -284,10 +308,17 @@ async function createNext( nextInstance.on('destroy', () => { nextInstance = undefined + clearFixture() }) await nextInstance.setup(rootSpan) + // Lazy `// @gate` conditions read this fixture's resolved next.config. + // Registering the instance (not a snapshot) before `start()` keeps + // `skipStart` suites and rebuild flows working: nothing is resolved until + // a gate actually asks. See test/lib/gate/README.md. + registerFixture(nextInstance) + if (!opts.skipStart) { await rootSpan .traceChild('start next instance') @@ -338,16 +369,86 @@ export function nextTestSetup( } } + // A lazy `@force-gate` on the enclosing `describe` (e.g. `!cacheComponents`) + // gates the *build*, not just the test bodies: some fixtures can't build + // under the condition at all. Snapshot the describe's gates now, while the + // describe body is still being collected — the stack is empty by `beforeAll`. + // Suites that manage their own build (`skipStart`) are left untouched. + const describeGates = getActiveDescribeGates() + // Deploy's "build" is a remote deployment we can't gate this way, and suites + // that pass `skipStart` build manually — leave both to their own handling. + const buildForceGated = + !options.skipStart && !isNextDeploy && hasLazyForceGate(describeGates) + let next: NextInstance | undefined if (!skipped) { beforeAll(async () => { - next = await createNext(options) + if (!buildForceGated) { + next = await createNext(options) + return + } + // Try to decide the force-gate against the *source* fixture first, + // before paying for the fixture setup (which includes a dependency + // install when the run is isolated). The config resolver falls back to + // the repo's own `next` when the directory has no install, and the env + // mirrors what `getSpawnOpts` hands every fixture child process. An + // inline `files` object has no directory to resolve against, and any + // resolution failure (e.g. a config that imports from the fixture's + // own node_modules) falls through to the instance-based decision below. + if (typeof options.files === 'string') { + const config = await loadResolvedConfig({ + dir: options.files, + phase: isNextDev ? PHASE_DEVELOPMENT_SERVER : PHASE_PRODUCTION_BUILD, + env: { + ...process.env, + ...options.env, + NODE_ENV: (options.env?.NODE_ENV || + '') as NodeJS.ProcessEnv['NODE_ENV'], + PORT: '0', + __NEXT_TEST_MODE: 'e2e', + }, + }).catch(() => null) + const earlySkip = config && findLazyForceSkip(describeGates, config) + if (earlySkip) { + // No instance ever exists on this path, so register the resolved + // config directly for the per-test force-pass decisions. + registerFixture({ getResolvedConfig: async () => config }) + require('console').warn( + ` ⚠ suite build skipped by \`@force-gate ${earlySkip.source}\` ` + + `(decided from the source fixture; setup skipped)` + ) + return + } + } + // Set the fixture up (so its config is resolvable) without building, then + // resolve the force-gate. If it's false, skip the build entirely — the + // inherited gate makes every test force-pass, so nothing touches `next`. + const instance = await createNext({ ...options, skipStart: true }) + next = instance + const config = await instance.getResolvedConfig() + const forceSkip = findLazyForceSkip(describeGates, config) + if (forceSkip) { + require('console').warn( + ` ⚠ suite build skipped by \`@force-gate ${forceSkip.source}\`` + ) + return + } + try { + await instance.start() + } catch (err) { + await instance.destroy().catch(() => {}) + next = undefined + throw err + } }) afterAll(async () => { // Gracefully destroy the instance if `createNext` success. // If next instance is not available, it's likely beforeAll hook failed and unnecessarily throws another error // by attempting to destroy on undefined. await next?.destroy() + // The early force-skip path registers a config source without an + // instance (an instance clears itself on destroy). + if (!next) clearFixture() }) } diff --git a/test/lib/gate/README.md b/test/lib/gate/README.md new file mode 100644 index 000000000000..4d225729be74 --- /dev/null +++ b/test/lib/gate/README.md @@ -0,0 +1,252 @@ +# `@gate` — marking a test as known-failing, without lying about it + +`it.skip` is a dead end. Nothing tells you when the bug it was hiding gets +fixed, so the test stays skipped, then stays skipped after it would have passed, +and eventually rots. `// @gate` replaces it with a tripwire. + +```ts +// Blocked on the optimization that marks a route as fully static when no +// dynamic params are referenced in Server Components. +// @gate !cacheComponents +it('navigate to page with a lazily-generated static param', async () => { + // body unchanged +}) +``` + +The test **still runs**. Because `cacheComponents` is on for this fixture the +condition is false, so the failure is expected: the suite stays green and the run +logs + +``` + ⚠ gated test failed as expected (@gate !cacheComponents) +``` + +The day the underlying bug is fixed and the body starts passing, CI fails with + +``` +Gated test passed unexpectedly. + +This test is marked `// @gate !cacheComponents`, and that condition is currently +false, so the test was expected to fail — but it passed. +The gate is stale: delete the `// @gate !cacheComponents` pragma (and whatever +workaround came with it). +``` + +That inversion — condition false + test passes ⇒ **failure** — is the whole +feature. It is lifted from React's `@gate` +(`scripts/jest/setupTests.js`, `scripts/babel/transform-test-gate-pragma.js`), +including the expression grammar, so pragmas read the same in both repos. + +## `@gate` vs `@force-gate` + +`@gate` **runs** the body and inverts the expectation when the condition is +false (a passing body then fails as stale). `@force-gate` **skips** instead of +running — for a body that isn't worth attempting, giving up the tripwire in +exchange. + +| directive | condition | when false | when true | +| --- | --- | --- | --- | +| `// @gate ` | static or lazy | assert-fail (invert; stale if it passes) | run | +| `// @force-gate ` (static) | static | real Jest skip (`○ skipped`) at collection | run | +| `// @force-gate ` (lazy, per-test) | lazy | force-pass the test (skip the body) | run | +| `// @force-gate ` (lazy, on a `describe`) | lazy | skip the **build** and force-pass the suite | build + run | + +A **static** `@force-gate` (mode/bundler) is decided while tests are collected, +so it's a real `○ skipped`. A **lazy** `@force-gate` (resolved-config) can't be +known then, so it's decided at runtime once the fixture's config is resolvable: + +- On a `describe`, the fixture is set up but the **build is skipped** when the + condition is false — which is the point, since some fixtures can't build under + the condition at all (e.g. `revalidate` / `dynamic` route configs under Cache + Components). Nothing is asserted; every test force-passes. +- Because Jest can't turn a running test into `○ skipped`, a lazy force-gate + reports the test as **passed with a `⚠ skipped by @force-gate ` warning**, + not as skipped. A static force-gate keeps the real `○ skipped`. + +**Prefer `@gate` when the off state fails for a meaningful reason** — a flag +that changes the behavior of existing surface, where a pass would tell you the +gate is stale. For a new API the off state can only throw, which proves nothing +and costs real browser time, so tests of a new API should typically +`@force-gate` instead. `@force-gate` is also the only option when running the +body is impossible: prefetching is off in dev, deploy has no local build +output, the fixture can't build under the condition. + +Both forms work on `it`, `test`, `fit`, `describe`, and their `.only` variants. +A gate on a `describe` applies to every test inside it. Several pragmas may stack +on one call. (Build-skipping applies only to suites where `nextTestSetup` owns +the build — not `skipStart` suites — and to `start`/`dev`, not deploy.) + +## Conditions + +All condition names are declared in [`conditions.ts`](./conditions.ts) — a typo +fails the whole suite at collection time rather than silently disabling the gate. +There are two tiers: + +- **static** — the run's own shape (`dev`, `start`, `deploy`, `mode`, + `turbopack`, `rspack`, `webpack`, `bundler`, `react18`, `wasm`, `ci`), + semantic aliases for `!dev` that state the reason rather than the mode + (`prod`, `prefetching`), plus `FIXME` / `TODO`, which are always false. +- **lazy** — a predicate over the fixture's *resolved* `next.config` + (`cacheComponents`, `ppr`, `prefetchInlining`, `output`, …), read the first + time a gate asks for it. + +Lazy conditions read the resolved config and never `process.env`, because +`__NEXT_CACHE_COMPONENTS=true` (the `--experimental` shard) is only applied when +the fixture has not set `cacheComponents` itself, and because resolution implies +flags a fixture never mentions — `cacheComponents: true` alone turns on +`experimental.ppr` and `experimental.cachedNavigations`. A gate therefore stays +correct when a fixture's config changes or a CI shard's env var starts or stops +applying. + +Add conditions freely; the guidance for doing so is at the top of +`conditions.ts`. + +## Covering both states of an experiment: test axes + +On top of the dimensions the test matrix already has (mode, bundler, React +version), the suites run once plainly and once per *test axis* — a fixed, +lettered set of alternate flag configurations marked by `__NEXT_TEST_AXIS`. +(Today there is one axis, `A`, an alias for the `--experimental` / +`__NEXT_CACHE_COMPONENTS` run; see `scripts/run-jest.sh`.) A fixture can key +an experimental flag on an axis instead of pinning it — enabled by default, +disabled on that axis: + +```js +// next.config.js — pin every dimension except the one under test +const nextConfig = { + cacheComponents: true, + experimental: { + concurrentRouterQueue: process.env.__NEXT_TEST_AXIS !== 'A', + }, +} +``` + +paired with `// @gate concurrentRouterQueue` on the tests whose expectations +only hold with the flag on (`test/e2e/app-dir/concurrent-router-queue/`). A +plain run — including a local run with no special env — exercises the +feature, while the axis-A run covers the off state — and fails the day the +gated tests start passing. One suite, both states, no new CI job, and no +duplicated fixture. When the off state proves nothing (a new API that throws +or is inert when disabled), pair the keying with a lazy `@force-gate` +instead — `test/e2e/app-dir/use-offline/` — and the axis run skips the +fixture build entirely. + +Keep exactly **one** flag varying per fixture (pin the rest, like +`cacheComponents` above) so a red shard still attributes to a single +dimension. The gate itself keeps working either way — lazy conditions read the +*resolved* config, so they observe whatever the fixture decided, not how it +decided it. + +To reproduce the disabled (axis) state locally, set the marker the same way +CI does: + +```sh +__NEXT_TEST_AXIS=A NEXT_SKIP_ISOLATE=1 pnpm test-start-webpack test/e2e/app-dir/concurrent-router-queue/concurrent-router-queue.test.ts +``` + +## Expressions + +``` +// @gate !dev +// @gate mode === 'start' && !cacheComponents +// @gate !(turbopack || rspack) +// @gate output === 'export' +``` + +`!`, `&&`, `||`, `===`/`!==` (and `==`/`!=`), parentheses, string and boolean +literals. Values are coerced by truthiness in boolean position, so +`@gate prefetchInlining` works even though it resolves to +`false | {maxSize, maxBundleSize}`. + +## Conditional logic inside a body: `gate()` + +The pragma gates a whole test. For a body that should run under both states +but *assert differently*, import the runtime `gate()` — the same registry, +without the inversion: + +```ts +import { gate } from 'next-test-utils' + +it('renders the fallback', async () => { + if (await gate((conditions) => conditions.cacheComponents)) { + // PPR shell: the fallback is part of the prerender. + } else { + // fully dynamic: the fallback streams in. + } +}) +``` + +The function form mirrors React's `gate(flags => flags.enableFoo)` +(`scripts/jest/setupTests.js`), except it is imported rather than a global, +and async, because a lazy condition reads the booted fixture's resolved +config. A string is also accepted and evaluated in the pragma expression +language: `await gate('cacheComponents && !dev')`. Either way an undeclared +name throws, like a pragma. + +`gate()` also works where a pragma cannot attach (`it.each`). Prefer it over +branching on `process.env` for the same reason lazy conditions exist: the env +var is not what the fixture actually resolved. + +## How it works + +1. `pragma-transform.js` rewrites the pragma into + `_test_gate([{force,source}], 'it')(...)`. It is a line-oriented regex, not an + AST transform, so **only the `it(` line changes** and every other line keeps + its byte offsets — `toMatchInlineSnapshot()` is written back by line/column. +2. `jest-transformer.js` chains that rewrite in front of the SWC transformer + `next/jest` configures. `jest.config.js` wires it up with + `withGateTransformer()`. +3. `runtime.ts` installs `_test_gate` and evaluates conditions. A false + *static* `@gate` is known while tests are collected, so the test registers + through Jest's native `test.failing` and the inversion is Jest's own. A + lazy gate can't be decided until the fixture's config resolves, so those + tests wrap the body and invert the outcome at runtime. The `it`/`test` + globals are wrapped so a gate on a `describe` reaches the tests inside. +4. `state.ts` holds the fixture `createNext()` registered; + `NextInstance.getResolvedConfig()` resolves its config out of process (in + process, `loadConfig` would mutate the Jest worker's `process.env` from the + fixture's `.env` files). + +A suite with no lazy gate never resolves a config, so the cost is zero. + +## Limitations + +- A pragma the transform would not pick up is a **hard error**, not a no-op: + blank line in between, `it.each` / `it.failing`, a pragma inside a JSDoc + block. Reword prose comments that start with `@gate`. A pragma on a + *skipped* test (`it.skip`, `xit`, …) gets a dedicated error — gating a skip + is ambiguous, so either remove the skip and let the gate decide, or keep the + plain skip and drop the pragma. (A skip *without* a pragma is left alone.) +- A `describe`-level gate does not reach `it.each` tests (they bypass the + `it` wrapper) — branch inside the body with the runtime `gate()` instead. +- A gated-false body that *stalls* rather than throwing wastes the full Jest + timeout. Under a static gate (native `test.failing`) the timeout counts as + the expected failure, so the test passes — slowly; under a lazy gate the + runtime inversion only absorbs thrown errors, so the timeout fails the suite + anyway. In practice `createRouterAct` and Playwright fail fast instead of + stalling. +- Only the test body is gated. A failure from an `afterEach` (e.g. the redbox + matchers) still fails the test. +- `jest.retryTimes(1)` is on for non-dev CI. A stale gate fails deterministically + on both attempts, but a *flaky* gated-false test now "passes" whenever it + happens to fail. +- A gated test's title is unchanged (React renames its to + `[GATED, SHOULD FAIL] …`; we can't, because a lazy gate is not decided when + titles are fixed). The `⚠ gated test failed as expected` line is the only + signal in the log today. + +## Tests + +`test/unit/gate/` covers the transform, the expression language, and the runtime. +The stale-gate *failure* cannot be asserted from inside Jest — a test that must +fail cannot report itself as passing — so it is verified by hand: + +```sh +# add `// @gate dev` above a test that passes in start mode, then: +NEXT_SKIP_ISOLATE=1 pnpm test-start test/e2e/app-dir/segment-cache/basic +# => FAIL … Gated test passed unexpectedly … The gate is stale +``` + +A child-process harness that automates this (the pattern React uses in +`scripts/babel/__tests__/transform-test-gate-pragma-test.js`) is a worthwhile +follow-up. diff --git a/test/lib/gate/conditions.ts b/test/lib/gate/conditions.ts new file mode 100644 index 000000000000..f028cf748439 --- /dev/null +++ b/test/lib/gate/conditions.ts @@ -0,0 +1,207 @@ +/** + * The `@gate` condition registry. + * + * Every name that may appear inside a `// @gate` / `// @force-gate` pragma has + * to be declared here. Referencing an undeclared name fails the whole test + * suite at collection time, so a typo can never silently disable a gate. + * + * The registry is deliberately hand-written rather than derived from the + * `next.config` schema: a gate is a claim about which *test-matrix dimension* + * explains a failure, and that claim is worth spelling out. Keep the list small + * and meaningful. + * + * ## The two tiers + * + * **`staticCondition`** — the value is known before any test runs (run mode, + * bundler, React version). These are the only conditions `@force-gate` accepts, + * because a real Jest skip has to be decided while tests are being collected. + * + * **`lazyCondition`** — a predicate over the *resolved* `next.config` of the + * fixture the suite booted (`NextInstance.getResolvedConfig()`). The value is + * read the first time a gate asks for it, which is inside the test body, + * because nothing about the fixture exists at collection time. + * + * Read the resolved config, never `process.env`: `__NEXT_CACHE_COMPONENTS=true` + * (set by the `--experimental` shard in `scripts/run-jest.sh`) is only applied + * when the fixture has not set `cacheComponents` itself, and resolution implies + * flags a fixture never mentions — `cacheComponents: true` alone turns on + * `experimental.ppr` and `experimental.cachedNavigations`. + * + * ## Adding a condition + * + * 1. Pick a bare name that reads well after `@gate` and `!`. + * 2. Add it below with a one-line description of what it means. + * 3. For a lazy condition, read the key off the resolved config — remember that + * some keys moved out of `experimental` (`config.cacheComponents`) while + * others are still under it (`config.experimental.ppr`), and that some + * normalize to an object rather than a boolean + * (`experimental.prefetchInlining`). Returning the raw value is fine: + * expressions coerce by truthiness, and `===` comparisons see the raw value. + * + * Values do not have to be booleans. `mode` and `bundler` are strings so that + * `// @gate mode === 'deploy'` works. + */ + +import type { ResolvedNextConfig } from './resolved-config' +import { getGateTestContext } from './test-context' + +export type ConditionValue = unknown + +export type StaticCondition = { + kind: 'static' + description: string + value: () => ConditionValue +} + +export type LazyCondition = { + kind: 'lazy' + description: string + value: (config: ResolvedNextConfig) => ConditionValue +} + +export type Condition = StaticCondition | LazyCondition + +function staticCondition( + description: string, + value: () => ConditionValue +): StaticCondition { + return { kind: 'static', description, value } +} + +function lazyCondition( + description: string, + value: (config: ResolvedNextConfig) => ConditionValue +): LazyCondition { + return { kind: 'lazy', description, value } +} + +export const conditions: Record = { + // --- static: the shape of this test run ----------------------------------- + + mode: staticCondition( + "the e2e run mode: 'dev' | 'start' | 'deploy'", + () => getGateTestContext().mode + ), + dev: staticCondition( + 'running `next dev`', + () => getGateTestContext().mode === 'dev' + ), + start: staticCondition( + 'running `next build` + `next start`', + () => getGateTestContext().mode === 'start' + ), + deploy: staticCondition( + 'running against a real deployment', + () => getGateTestContext().mode === 'deploy' + ), + + // Semantic aliases for `!dev`. A gate is a claim about *why* a suite cannot + // run, so prefer the name that states the reason over the bare mode check. + prod: staticCondition( + 'the app was built with `next build` (`start` or `deploy`)', + () => getGateTestContext().mode !== 'dev' + ), + prefetching: staticCondition( + 'links prefetch — disabled in dev, the usual reason a suite skips it', + () => getGateTestContext().mode !== 'dev' + ), + + bundler: staticCondition( + "the bundler under test: 'turbopack' | 'rspack' | 'webpack'", + () => getGateTestContext().bundler + ), + turbopack: staticCondition( + 'bundling with Turbopack', + () => getGateTestContext().bundler === 'turbopack' + ), + rspack: staticCondition( + 'bundling with Rspack', + () => getGateTestContext().bundler === 'rspack' + ), + webpack: staticCondition( + 'bundling with webpack', + () => getGateTestContext().bundler === 'webpack' + ), + + react18: staticCondition( + 'the fixture installs React 18 instead of the default React version', + () => getGateTestContext().react18 + ), + wasm: staticCondition( + 'using the wasm SWC binary (`NEXT_TEST_WASM`)', + () => getGateTestContext().wasm + ), + ci: staticCondition('running in CI (`NEXT_TEST_CI`)', () => + Boolean(process.env.NEXT_TEST_CI) + ), + + // Always false, so `// @gate FIXME` marks a test as a known failure without + // inventing a condition for it. Mirrors the same convention in React's + // scripts/jest/TestFlags.js. Prefer a real condition whenever one exists — + // these two say "we know this is broken" and nothing about why. + FIXME: staticCondition('known failure, no condition attached', () => false), + TODO: staticCondition('known failure, no condition attached', () => false), + + // --- lazy: the fixture's resolved next.config ------------------------------ + + cacheComponents: lazyCondition( + 'Cache Components are enabled for the fixture', + (config) => config.cacheComponents + ), + ppr: lazyCondition( + 'partial prerendering is enabled (implied by `cacheComponents`)', + (config) => config.experimental?.ppr + ), + cachedNavigations: lazyCondition( + 'client navigations are cached (implied by `cacheComponents`)', + (config) => config.experimental?.cachedNavigations + ), + optimisticRouting: lazyCondition( + 'optimistic routing is enabled', + (config) => config.experimental?.optimisticRouting + ), + concurrentRouterQueue: lazyCondition( + 'the concurrent router queue fork is enabled', + (config) => config.experimental?.concurrentRouterQueue + ), + dynamicOnHover: lazyCondition( + 'dynamic prefetches are triggered on hover', + (config) => config.experimental?.dynamicOnHover + ), + useOffline: lazyCondition( + 'the `useOffline()` hook is enabled for the fixture', + (config) => config.experimental?.useOffline + ), + prefetchInlining: lazyCondition( + 'prefetches are inlined into the HTML payload; resolves to an object ' + + '(`{maxSize, maxBundleSize}`) or `false`', + (config) => config.experimental?.prefetchInlining + ), + output: lazyCondition( + "`output` in the resolved config: 'standalone' | 'export' | undefined", + (config) => config.output + ), + basePath: lazyCondition( + 'the fixture serves the app from a base path (a string, `` when unset)', + (config) => config.basePath + ), + trailingSlash: lazyCondition( + 'URLs are normalized to a trailing slash', + (config) => config.trailingSlash + ), +} + +export function isDeclared(name: string): boolean { + return Object.prototype.hasOwnProperty.call(conditions, name) +} + +export function getCondition(name: string): Condition { + if (!isDeclared(name)) { + throw new Error( + `\`@gate\` references an undeclared condition "${name}".\n\n` + + `Declare it in test/lib/gate/conditions.ts, or fix the typo. ` + + `Declared conditions: ${Object.keys(conditions).sort().join(', ')}.` + ) + } + return conditions[name] +} diff --git a/test/lib/gate/expr.ts b/test/lib/gate/expr.ts new file mode 100644 index 000000000000..a52a9056edec --- /dev/null +++ b/test/lib/gate/expr.ts @@ -0,0 +1,232 @@ +/** + * The tiny expression language used inside a `// @gate` pragma. + * + * ``` + * expression → binary ( ( "||" | "&&" ) binary )* ; + * binary → unary ( ( "==" | "!=" | "===" | "!==" ) unary )* ; + * unary → "!" unary | primary ; + * primary → NAME | STRING | BOOLEAN | "(" expression ")" ; + * ``` + * + * This mirrors the grammar React uses for its own `@gate` pragmas + * (`scripts/babel/transform-test-gate-pragma.js` in facebook/react), so + * pragmas read the same in both repos: + * + * ``` + * // @gate !dev + * // @gate mode === 'start' && !cacheComponents + * ``` + * + * `NAME` is a condition declared in `./conditions.ts`. Unlike React, the + * expression is parsed at *runtime* rather than compiled by the transform, + * which keeps the source rewrite trivial and lets the runtime report the + * pragma text verbatim in error messages. + */ + +export type ExprNode = + | { type: 'literal'; value: string | boolean } + | { type: 'condition'; name: string } + | { type: 'not'; argument: ExprNode } + | { type: 'logical'; op: '&&' | '||'; left: ExprNode; right: ExprNode } + | { type: 'compare'; op: '=='; left: ExprNode; right: ExprNode } + | { type: 'compare'; op: '!='; left: ExprNode; right: ExprNode } + +export type ParsedExpression = { + node: ExprNode + /** Every condition name referenced by the expression, deduplicated. */ + names: string[] +} + +type Token = + | { type: 'name'; name: string } + | { type: 'string'; value: string } + | { type: 'boolean'; value: boolean } + | { type: '&&' | '||' | '==' | '!=' | '!' | '(' | ')' } + +const NAME_RE = /[a-zA-Z_$][0-9a-zA-Z_$]*/y + +function tokenize(source: string): Token[] { + const tokens: Token[] = [] + let i = 0 + while (i < source.length) { + const char = source[i] + + if (char === '"' || char === "'") { + let value = '' + i++ + while (i < source.length && source[i] !== char) value += source[i++] + if (source[i] !== char) { + throw new SyntaxError( + `Unterminated string in \`${source}\` (missing closing ${char}).` + ) + } + i++ + tokens.push({ type: 'string', value }) + continue + } + + if (/\s/.test(char)) { + i++ + continue + } + + const next3 = source.slice(i, i + 3) + if (next3 === '===') { + tokens.push({ type: '==' }) + i += 3 + continue + } + if (next3 === '!==') { + tokens.push({ type: '!=' }) + i += 3 + continue + } + + const next2 = source.slice(i, i + 2) + if (next2 === '&&' || next2 === '||' || next2 === '==' || next2 === '!=') { + tokens.push({ type: next2 }) + i += 2 + continue + } + + if (char === '(' || char === ')' || char === '!') { + tokens.push({ type: char }) + i++ + continue + } + + NAME_RE.lastIndex = i + const match = NAME_RE.exec(source) + if (match) { + const name = match[0] + if (name === 'true' || name === 'false') { + tokens.push({ type: 'boolean', value: name === 'true' }) + } else { + tokens.push({ type: 'name', name }) + } + i += name.length + continue + } + + throw new SyntaxError( + `Unexpected character ${JSON.stringify(char)} in \`${source}\`.` + ) + } + return tokens +} + +/** Parses a pragma condition, collecting the condition names it references. */ +export function parse(source: string): ParsedExpression { + const tokens = tokenize(source) + const names = new Set() + let i = 0 + + function expression(): ExprNode { + let left = binary() + for (;;) { + const token = tokens[i] + if (token && (token.type === '&&' || token.type === '||')) { + i++ + left = { type: 'logical', op: token.type, left, right: binary() } + continue + } + return left + } + } + + function binary(): ExprNode { + let left = unary() + for (;;) { + const token = tokens[i] + if (token && (token.type === '==' || token.type === '!=')) { + i++ + left = { type: 'compare', op: token.type, left, right: unary() } + continue + } + return left + } + } + + function unary(): ExprNode { + if (tokens[i]?.type === '!') { + i++ + return { type: 'not', argument: unary() } + } + return primary() + } + + function primary(): ExprNode { + const token = tokens[i] + if (!token) { + throw new SyntaxError(`Unexpected end of expression in \`${source}\`.`) + } + switch (token.type) { + case 'boolean': + case 'string': + i++ + return { type: 'literal', value: token.value } + case 'name': + i++ + names.add(token.name) + return { type: 'condition', name: token.name } + case '(': { + i++ + const inner = expression() + if (tokens[i]?.type !== ')') { + throw new SyntaxError(`Missing closing \`)\` in \`${source}\`.`) + } + i++ + return inner + } + default: + throw new SyntaxError(`Unexpected \`${token.type}\` in \`${source}\`.`) + } + } + + const node = expression() + if (i !== tokens.length) { + throw new SyntaxError( + `Unexpected \`${tokens[i].type}\` after a complete expression in ` + + `\`${source}\`.` + ) + } + return { node, names: [...names] } +} + +function evaluateNode( + node: ExprNode, + read: (name: string) => unknown +): unknown { + switch (node.type) { + case 'literal': + return node.value + case 'condition': + return read(node.name) + case 'not': + return !evaluateNode(node.argument, read) + case 'logical': + return node.op === '&&' + ? evaluateNode(node.left, read) && evaluateNode(node.right, read) + : evaluateNode(node.left, read) || evaluateNode(node.right, read) + case 'compare': { + const left = evaluateNode(node.left, read) + const right = evaluateNode(node.right, read) + return node.op === '==' ? left === right : left !== right + } + default: + throw new Error(`Unknown @gate expression node: ${JSON.stringify(node)}`) + } +} + +/** + * Evaluates a parsed expression. Condition values are coerced by truthiness in + * boolean position, so `@gate prefetchInlining` works for a condition whose + * value is `false | {maxSize: number}`, and `@gate output === 'export'` works + * for string-valued conditions. + */ +export function evaluate( + node: ExprNode, + read: (name: string) => unknown +): boolean { + return Boolean(evaluateNode(node, read)) +} diff --git a/test/lib/gate/jest-transformer.js b/test/lib/gate/jest-transformer.js new file mode 100644 index 000000000000..554c1e9b4945 --- /dev/null +++ b/test/lib/gate/jest-transformer.js @@ -0,0 +1,100 @@ +// @ts-check + +/** + * Jest transformer that rewrites `// @gate` pragmas (see + * `./pragma-transform.js`) and then delegates to the transformer `next/jest` + * would have used on its own (SWC). + * + * It is wired up by `jest.config.js` via `withGateTransformer()`, which takes + * the transformer entry `next/jest` produced and nests it inside this one, so + * there is still exactly one source of truth for the SWC options. + */ + +const crypto = require('crypto') +const fs = require('fs') +const path = require('path') + +const { rewrite } = require('./pragma-transform') + +const IS_TEST_FILE = /\.test\.(js|jsx|ts|tsx|mjs)$/ + +/** The transform key `next/jest` uses for its SWC transformer. */ +const TRANSFORM_KEY = '^.+\\.(js|jsx|ts|tsx|mjs)$' + +/** + * Hash of this transformer's own sources, mixed into `getCacheKey` so that + * editing the pragma rewrite invalidates Jest's transform cache. Jest's + * built-in fallback only hashes the file contents and the serialized config, + * neither of which changes when this directory does. + */ +const SELF_VERSION = (() => { + const hash = crypto.createHash('sha1') + for (const file of ['jest-transformer.js', 'pragma-transform.js']) { + hash.update(fs.readFileSync(path.join(__dirname, file))) + } + return hash.digest('hex').slice(0, 16) +})() + +/** + * @typedef {{ innerTransformer: string, innerOptions: unknown }} GateTransformerConfig + */ + +/** @type {(inputOptions: GateTransformerConfig) => import('@jest/transform').SyncTransformer} */ +function createTransformer(inputOptions) { + if (!inputOptions?.innerTransformer) { + throw new Error( + 'test/lib/gate/jest-transformer.js must be configured through ' + + '`withGateTransformer()` in jest.config.js.' + ) + } + const innerModule = require(inputOptions.innerTransformer) + const inner = innerModule.createTransformer(inputOptions.innerOptions) + + return { + process(src, filename, jestOptions) { + const rewritten = IS_TEST_FILE.test(filename) + ? rewrite(src, filename) + : src + return inner.process(rewritten, filename, jestOptions) + }, + getCacheKey(src, filename, options) { + const base = inner.getCacheKey + ? inner.getCacheKey(src, filename, options) + : crypto.createHash('sha1').update(src).update(filename).digest('hex') + return `${base}:gate-${SELF_VERSION}` + }, + } +} + +/** + * Wraps the transformer entry produced by `next/jest` so `@gate` pragmas are + * rewritten before SWC compiles the file. + * + * @template {{ transform?: Record }} T + * @param {T} config a resolved Jest config from `next/jest` + * @returns {T} + */ +function withGateTransformer(config) { + const existing = config.transform?.[TRANSFORM_KEY] + if (!Array.isArray(existing) || typeof existing[0] !== 'string') { + throw new Error( + `withGateTransformer: expected next/jest to define a transformer tuple ` + + `for ${TRANSFORM_KEY}, found ${JSON.stringify(existing)}. ` + + `next/jest's transform shape changed — update ` + + `test/lib/gate/jest-transformer.js.` + ) + } + const [innerTransformer, innerOptions] = existing + return { + ...config, + transform: { + ...config.transform, + [TRANSFORM_KEY]: [ + require.resolve('./jest-transformer.js'), + { innerTransformer, innerOptions }, + ], + }, + } +} + +module.exports = { createTransformer, withGateTransformer, TRANSFORM_KEY } diff --git a/test/lib/gate/load-config-child.js b/test/lib/gate/load-config-child.js new file mode 100644 index 000000000000..1ea498554dd4 --- /dev/null +++ b/test/lib/gate/load-config-child.js @@ -0,0 +1,88 @@ +// @ts-check + +/** + * Resolves a fixture's `next.config` and prints it as JSON. + * + * Run as a child process by `./load-resolved-config.ts`, with the fixture's cwd + * and the fixture's exact spawn env. It has to be out of process for two + * reasons: + * + * 1. `loadConfig` calls `loadEnvConfig`, which **mutates the caller's + * `process.env`** from the fixture's `.env*` files. Doing that inside a Jest + * worker would leak fixture env into every other test in the file. + * 2. Resolution reads env vars from the *calling* process + * (`__NEXT_CACHE_COMPONENTS`, `__NEXT_EXPERIMENTAL_CACHED_NAVIGATIONS`, ...), + * and the fixture runs with an env that is not the Jest worker's whenever a + * suite passes `nextTestSetup({ env })`. + * + * Usage: node load-config-child.js + */ + +const MARKER = '__NEXT_GATE_RESOLVED_CONFIG__' + +/** + * Deep-copies `value` into something `JSON.stringify` can handle: drops + * functions and symbols, stringifies regexps, and replaces cycles with + * `'[Circular]'`. Anything a `@gate` condition wants to read is plain data. + * + * @param {unknown} value + * @param {WeakSet} [seen] + * @returns {unknown} + */ +function toJsonSafe(value, seen = new WeakSet()) { + if (value === null || typeof value !== 'object') { + if (typeof value === 'function' || typeof value === 'symbol') { + return undefined + } + if (typeof value === 'bigint') return String(value) + return value + } + if (value instanceof RegExp) return String(value) + if (value instanceof Date) return value.toISOString() + if (seen.has(value)) return '[Circular]' + seen.add(value) + if (Array.isArray(value)) { + return value.map((item) => toJsonSafe(item, seen)) + } + /** @type {Record} */ + const result = {} + for (const [key, item] of Object.entries(value)) { + const converted = toJsonSafe(item, seen) + if (converted !== undefined) result[key] = converted + } + return result +} + +/** + * Prefer the `next` the fixture itself installed, so the config is resolved by + * the same code the fixture builds with. + * + * @param {string} dir + */ +function requireConfigLoader(dir) { + try { + return require(require.resolve('next/dist/server/config', { paths: [dir] })) + } catch { + return require('next/dist/server/config') + } +} + +async function main() { + const [dir, phase] = process.argv.slice(2) + if (!dir || !phase) { + throw new Error('usage: load-config-child.js ') + } + const loadConfig = requireConfigLoader(dir).default + const config = await loadConfig(phase, dir, { silent: true }) + process.stdout.write(MARKER + JSON.stringify(toJsonSafe(config))) +} + +// Guarded so the parent can `require` this file just to read `MARKER`. +if (require.main === module) { + main().catch((err) => { + console.error(err) + process.exit(1) + }) +} + +module.exports = { MARKER, toJsonSafe } diff --git a/test/lib/gate/load-resolved-config.ts b/test/lib/gate/load-resolved-config.ts new file mode 100644 index 000000000000..3e1789602f27 --- /dev/null +++ b/test/lib/gate/load-resolved-config.ts @@ -0,0 +1,75 @@ +import spawn from 'cross-spawn' +import path from 'path' + +import type { ResolvedNextConfig } from './resolved-config' + +const { MARKER } = require('./load-config-child.js') + +const CHILD_SCRIPT = path.join(__dirname, 'load-config-child.js') +const TIMEOUT_MS = 60 * 1000 + +export type LoadResolvedConfigOptions = { + /** The fixture's test directory. */ + dir: string + /** `PHASE_PRODUCTION_BUILD` / `PHASE_DEVELOPMENT_SERVER`. */ + phase: string + /** The fixture's spawn env, from `NextInstance.getSpawnOpts()`. */ + env?: NodeJS.ProcessEnv +} + +/** + * Resolves a fixture's `next.config` in a child process. Costs ~0.15s, and is + * only ever paid by a suite that actually has a lazy `@gate`. + */ +export function loadResolvedConfig({ + dir, + phase, + env, +}: LoadResolvedConfigOptions): Promise { + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [CHILD_SCRIPT, dir, phase], { + cwd: dir, + env, + stdio: ['ignore', 'pipe', 'pipe'], + }) + + let stdout = '' + let stderr = '' + child.stdout!.on('data', (chunk) => (stdout += chunk)) + child.stderr!.on('data', (chunk) => (stderr += chunk)) + + const timeout = setTimeout(() => { + child.kill('SIGKILL') + reject( + new Error( + `Timed out after ${TIMEOUT_MS}ms resolving the next.config of ${dir} ` + + `for a \`@gate\` condition.` + ) + ) + }, TIMEOUT_MS) + + child.on('error', (error) => { + clearTimeout(timeout) + reject(error) + }) + + child.on('close', (code) => { + clearTimeout(timeout) + const start = stdout.indexOf(MARKER) + if (code !== 0 || start === -1) { + reject( + new Error( + `Failed to resolve the next.config of ${dir} for a \`@gate\` ` + + `condition (phase ${phase}, exit code ${code}).\n${stderr}` + ) + ) + return + } + try { + resolve(JSON.parse(stdout.slice(start + MARKER.length))) + } catch (error) { + reject(error) + } + }) + }) +} diff --git a/test/lib/gate/pragma-transform.js b/test/lib/gate/pragma-transform.js new file mode 100644 index 000000000000..62594f9b91aa --- /dev/null +++ b/test/lib/gate/pragma-transform.js @@ -0,0 +1,201 @@ +// @ts-check + +/** + * Source-to-source rewrite of `// @gate` / `// @force-gate` pragmas. + * + * ``` + * // @gate !cacheComponents + * it('does a thing', async () => {}) + * ``` + * + * becomes + * + * ``` + * // @gate !cacheComponents + * _test_gate([{"force":false,"source":"!cacheComponents"}],"it")('does a thing', async () => {}) + * ``` + * + * `_test_gate` is installed as a global by `test/lib/gate/runtime.ts` and + * returns a curried `it`-alike, so the original `(name, fn, timeout)` arguments + * flow through untouched. + * + * This is deliberately a regex rewrite and not an AST transform: the only + * edited bytes are on the `it(` / `describe(` line itself, so the line numbers + * of every other line are preserved exactly. That matters because Jest writes + * `toMatchInlineSnapshot()` results back by line/column, and because stack + * traces should still point at the original source. + */ + +/** + * A pragma block is one or more consecutive `// @gate` lines *immediately* + * followed by an `it` / `test` / `fit` / `describe` call. Anything else (a + * pragma in a JSDoc block, a pragma with a blank line under it, a pragma over + * `it.each`) is not matched, and is reported as an error by `rewrite()` rather + * than silently ignored. + */ +const PRAGMA_BLOCK = + /((?:^[ \t]*\/\/[ \t]*@(?:force-)?gate\b[^\n]*\n)+)([ \t]*)(it|test|fit|describe)((?:\.only)?)([ \t]*\()/gm + +/** Matches a single pragma line and captures `force` + the condition source. */ +const PRAGMA_LINE = /^[ \t]*\/\/[ \t]*@(force-)?gate\b[ \t]*([^\n]*)$/ + +/** Cheap detector used to find pragmas the block regex did not consume. */ +const PRAGMA_LINE_LOOSE = /^[ \t]*\/\/[ \t]*@(?:force-)?gate\b/ + +/** + * A skipped (or todo) test call. A skip *without* a pragma flows through + * untouched, but a pragma on one is ambiguous — should the gate re-enable the + * test, or does the skip win? — so it gets a dedicated error instead of the + * generic misplaced-pragma one. + */ +const SKIPPED_CALL = + /^[ \t]*(?:xit|xtest|xdescribe|(?:it|test|describe)\.skip|(?:it|test)\.todo)[ \t]*[(.]/ + +/** + * @param {string} src + * @returns {number[]} 0-based index of the first character of each line + */ +function getLineStarts(src) { + const starts = [0] + for (let i = 0; i < src.length; i++) { + if (src[i] === '\n') starts.push(i + 1) + } + return starts +} + +/** + * @param {number[]} lineStarts + * @param {number} offset + * @returns {number} 1-based line number containing `offset` + */ +function getLineNumber(lineStarts, offset) { + let low = 0 + let high = lineStarts.length - 1 + while (low < high) { + const mid = (low + high + 1) >> 1 + if (lineStarts[mid] <= offset) low = mid + else high = mid - 1 + } + return low + 1 +} + +/** + * Rewrites every `@gate` / `@force-gate` pragma block in `src`. + * + * Throws if the file contains a pragma-looking comment that would not have any + * effect — a silently-inert gate is the worst possible failure mode for a + * feature whose entire purpose is to not lie about coverage. + * + * @param {string} src + * @param {string} [filename] + * @returns {string} + */ +function rewrite(src, filename) { + // Cheap bail-out for the ~2000 files with no pragma. `@force-gate` does not + // contain the substring `@gate`, so both spellings have to be checked. + if (!src.includes('@gate') && !src.includes('@force-gate')) return src + + const lineStarts = getLineStarts(src) + /** @type {Set} 1-based line numbers consumed by a match */ + const consumed = new Set() + + const output = src.replace( + PRAGMA_BLOCK, + /** + * @param {string} _all + * @param {string} pragmaLines + * @param {string} indent + * @param {string} callee + * @param {string} only + * @param {string} openParen + * @param {number} offset + */ + (_all, pragmaLines, indent, callee, only, openParen, offset) => { + const firstLine = getLineNumber(lineStarts, offset) + const gates = [] + const lines = pragmaLines.split('\n') + // The trailing element is '' because `pragmaLines` ends with a newline. + for (let i = 0; i < lines.length - 1; i++) { + consumed.add(firstLine + i) + const match = PRAGMA_LINE.exec(lines[i]) + if (!match) { + // Not reachable: PRAGMA_BLOCK already matched these lines. + throw new Error(`Unparsable @gate pragma: ${lines[i]}`) + } + const source = match[2].trim() + if (!source) { + throw new Error( + `${describeLocation(filename, firstLine + i)}: \`@${ + match[1] ? 'force-gate' : 'gate' + }\` needs a condition, e.g. \`// @gate !cacheComponents\`.` + ) + } + gates.push({ force: Boolean(match[1]), source }) + } + + return ( + pragmaLines + + indent + + `_test_gate(${JSON.stringify(gates)},${JSON.stringify( + callee + only + )})` + + openParen + ) + } + ) + + assertNoInertPragmas(src, consumed, filename) + + return output +} + +/** + * @param {string} src + * @param {Set} consumed + * @param {string | undefined} filename + */ +function assertNoInertPragmas(src, consumed, filename) { + const lines = src.split('\n') + for (let i = 0; i < lines.length; i++) { + if (!PRAGMA_LINE_LOOSE.test(lines[i])) continue + if (consumed.has(i + 1)) continue + + // Walk past the rest of the pragma block to the line it tried to gate. + let target = i + 1 + while (target < lines.length && PRAGMA_LINE_LOOSE.test(lines[target])) { + target++ + } + if (target < lines.length && SKIPPED_CALL.test(lines[target])) { + throw new Error( + `${describeLocation(filename, i + 1)}: a \`@gate\` pragma on a ` + + `skipped test is ambiguous.\n\n` + + ` ${lines[i].trim()}\n ${lines[target].trim()}\n\n` + + `Either remove the skip and let the gate decide (a false condition ` + + `absorbs the failure), or keep the plain skip and remove the ` + + `pragma.` + ) + } + + throw new Error( + `${describeLocation(filename, i + 1)}: this \`@gate\` pragma has no ` + + `effect.\n\n` + + ` ${lines[i].trim()}\n\n` + + `A pragma must sit on the line(s) immediately above an ` + + `\`it(\`, \`test(\`, \`fit(\`, \`describe(\`, \`it.only(\`, ` + + `\`test.only(\` or \`describe.only(\` call — with no blank line in ` + + `between. \`it.each\` and \`it.failing\` are not supported. If this ` + + `comment is prose rather than a pragma, reword it so it does not ` + + `start with \`@gate\`.` + ) + } +} + +/** + * @param {string | undefined} filename + * @param {number} line + */ +function describeLocation(filename, line) { + return `${filename ?? ''}:${line}` +} + +module.exports = { rewrite } diff --git a/test/lib/gate/resolved-config.ts b/test/lib/gate/resolved-config.ts new file mode 100644 index 000000000000..ad6a94c773e6 --- /dev/null +++ b/test/lib/gate/resolved-config.ts @@ -0,0 +1,13 @@ +/** + * A JSON-safe snapshot of a fixture's resolved `NextConfigComplete`, as + * produced by `NextInstance.getResolvedConfig()`. + * + * It is intentionally loosely typed. The snapshot round-trips through JSON, so + * functions (`webpack`, `generateBuildId`) and other non-serializable values + * are gone, and lazy conditions in `./conditions.ts` may want to read keys that + * only exist after resolution and are absent from the public `NextConfig` type. + */ +export type ResolvedNextConfig = { + experimental?: Record + [key: string]: unknown +} diff --git a/test/lib/gate/runtime.ts b/test/lib/gate/runtime.ts new file mode 100644 index 000000000000..bf04fe41a224 --- /dev/null +++ b/test/lib/gate/runtime.ts @@ -0,0 +1,524 @@ +/** + * The `@gate` runtime. + * + * `test/lib/gate/pragma-transform.js` rewrites + * + * ``` + * // @gate !cacheComponents + * it('name', body) + * ``` + * + * into `_test_gate([{force: false, source: '!cacheComponents'}], 'it')('name', body)`, + * and this module installs that `_test_gate` global. + * + * ## What a gate does + * + * `// @gate ` **always runs the test**. If the condition is true the + * test behaves normally. If the condition is false the expectation is inverted: + * a failing body is reported as a pass, and a *passing* body is reported as a + * failure — "the gate is stale, delete it". That inversion is the whole point. + * An `it.skip` is a dead end that nobody revisits; a `@gate` is a tripwire that + * fires the day the underlying bug is fixed. + * + * Conditions are declared in `./conditions.ts`. Static ones (mode, bundler) + * are known at collection time, so a false static `@gate` registers the test + * through Jest's own `test.failing` and the inversion is native. Lazy ones are + * read from the booted fixture's resolved config the first time a gate asks, + * which can only happen inside the test body — those tests wrap the body and + * invert the outcome at runtime. + * + * ## `@force-gate` + * + * `// @force-gate ` skips the test for real (`○ skipped`) when the + * condition is false. That requires a decision at collection time, so it only + * accepts static conditions, and it gives up the stale-gate tripwire entirely. + * Prefer `@gate`; reach for `@force-gate` only when running the body is + * impossible rather than merely failing — dev mode has no build output, deploy + * mode cannot touch the filesystem. + */ + +import { evaluate, parse, type ExprNode } from './expr' +import { getCondition } from './conditions' +import { getResolvedConfigForGates, hasFixture } from './state' +import type { ResolvedNextConfig } from './resolved-config' + +/** The shape the transform emits. */ +export type GatePragma = { + force: boolean + source: string +} + +export type Gate = GatePragma & { + node: ExprNode + names: string[] + /** True when any referenced condition has to be read off the fixture. */ + needsResolvedConfig: boolean +} + +type TestFn = ( + name: string, + fn?: jest.ProvidesCallback, + timeout?: number +) => void + +/** + * Gates on an enclosing `describe` apply to every test inside it, including + * tests that carry no pragma of their own. The stack is pushed while the + * `describe` body is being collected. + */ +const describeGateStack: Gate[] = [] + +/** Bodies this module already wrapped, so inherited gates are not re-applied. */ +const gatedBodies = new WeakSet() + +function staleGateMessage(gate: Gate): string { + return ( + `Gated test passed unexpectedly.\n\n` + + `This test is marked \`// @gate ${gate.source}\`, and that condition is ` + + `currently false, so the test was expected to fail — but it passed.\n` + + `The gate is stale: delete the \`// @gate ${gate.source}\` pragma (and ` + + `whatever workaround came with it).` + ) +} + +function parseGate(pragma: GatePragma): Gate { + let parsed + try { + parsed = parse(pragma.source) + } catch (err) { + throw new Error( + `Could not parse \`// @${pragma.force ? 'force-gate' : 'gate'} ${ + pragma.source + }\`: ${(err as Error).message}` + ) + } + + const lazyNames = parsed.names.filter( + (name) => getCondition(name).kind === 'lazy' + ) + + // `needsResolvedConfig` also classifies a `@force-gate`. A static force-gate + // (mode/bundler) is decided while tests are collected — a real Jest skip. A + // *lazy* force-gate can't be known then, so it is decided at runtime once the + // fixture's config is resolvable: it force-passes the test (and, on a + // `describe`, skips the build) rather than emitting a collection-time skip. + return { ...pragma, ...parsed, needsResolvedConfig: lazyNames.length > 0 } +} + +function readCondition(name: string, config?: ResolvedNextConfig): unknown { + const condition = getCondition(name) + if (condition.kind === 'static') return condition.value() + if (!config) { + // Unreachable: `needsResolvedConfig` makes us resolve the config first. + throw new Error(`\`@gate ${name}\` was evaluated without a config`) + } + return condition.value(config) +} + +type GateDecision = + | { type: 'run' } + | { type: 'force-pass'; gate: Gate } + | { type: 'invert'; gate: Gate } + +/** + * Decides what to do with a test, given the gates that apply to it. Resolves + * the fixture config once if any gate needs it. + * + * A false `@force-gate` skips the test (force-pass) and takes precedence over + * an inverted `@gate` on the same test — you can't assert-fail a test you're + * skipping. Otherwise the first false `@gate` inverts the expectation, and if + * every gate holds the test runs normally. + */ +async function decideGates(gates: Gate[]): Promise { + let config: ResolvedNextConfig | undefined + if (gates.some((gate) => gate.needsResolvedConfig)) { + config = await getResolvedConfigForGates() + } + const read = (name: string) => readCondition(name, config) + + for (const gate of gates) { + if (gate.force && !evaluate(gate.node, read)) { + return { type: 'force-pass', gate } + } + } + for (const gate of gates) { + if (!gate.force && !evaluate(gate.node, read)) { + return { type: 'invert', gate } + } + } + return { type: 'run' } +} + +/** The condition values a `gate()` predicate receives. */ +export type GateConditions = Record + +/** + * Reads conditions by name, validating against the registry (an undeclared + * name throws, same as a pragma). Lazy reads without a resolved config raise + * the standard "no fixture is registered" error. + */ +function makeConditionsObject( + config: ResolvedNextConfig | undefined +): GateConditions { + return new Proxy({} as GateConditions, { + get(_target, prop) { + if (typeof prop !== 'string') return undefined + const condition = getCondition(prop) + if (condition.kind === 'static') return condition.value() + if (config === undefined) { + // No fixture is registered (otherwise `gate()` resolved the config + // before calling the predicate); this throws the explanatory error. + void getResolvedConfigForGates() + } + return condition.value(config!) + }, + has: () => true, + }) +} + +/** + * The runtime counterpart of the pragma, for conditional logic *inside* a test + * body — the same condition registry, evaluated on demand: + * + * ```ts + * import { gate } from 'next-test-utils' + * + * if (await gate((conditions) => conditions.cacheComponents)) { + * expect(...).toBe(...) + * } else { + * expect(...).toBe(...) + * } + * ``` + * + * The function form mirrors React's `gate(flags => flags.enableFoo)` + * (`scripts/jest/setupTests.js`), except it is imported rather than installed + * as a global, and it is async, because lazy conditions read the booted + * fixture's resolved config. A string is also accepted and evaluated in the + * pragma expression language: `await gate('cacheComponents && !dev')`. + * + * Unlike the pragma there is no inversion — it only answers the question — so + * reach for it when a body should run under both states but assert + * differently, and for tests a pragma cannot attach to (`it.each`). When the + * whole body is a known failure, use `// @gate` instead and keep the tripwire. + */ +export async function gate( + arg: string | ((conditions: GateConditions) => unknown) +): Promise { + if (typeof arg === 'string') { + const parsed = parseGate({ force: false, source: arg }) + let config: ResolvedNextConfig | undefined + if (parsed.needsResolvedConfig) { + config = await getResolvedConfigForGates() + } + return Boolean(evaluate(parsed.node, (name) => readCondition(name, config))) + } + + // Whether the predicate reads a lazy condition can't be known without + // running it, so resolve the config up front whenever a fixture is + // registered (memoized on the instance). Suites without a fixture can still + // gate on static conditions. + const config = hasFixture() ? await getResolvedConfigForGates() : undefined + return Boolean(arg(makeConditionsObject(config))) +} + +/** + * Runs `callback` and throws `errorIfItPasses` if it *doesn't* fail. Adapted + * from React's `scripts/jest/setupTests.js`. + */ +export async function expectTestToFail( + callback: () => unknown, + errorIfItPasses: Error +): Promise { + let didError = false + try { + await callback() + } catch { + didError = true + } + if (!didError) throw errorIfItPasses +} + +function wrapGatedBody( + gates: Gate[], + callback: Function +): jest.ProvidesCallback { + if (callback.length > 0) { + throw new Error( + `A gated test cannot use the \`done\` callback, because the gate has to ` + + `observe whether the test failed. Return a promise instead.` + ) + } + + const body = async function gatedBody(this: unknown): Promise { + const decision = await decideGates(gates) + + if (decision.type === 'run') { + await callback.call(this) + return + } + + if (decision.type === 'force-pass') { + // A lazy `@force-gate` whose condition is false: the test can't be + // attempted here, so skip the body and report a pass. Jest can't turn a + // running test into `○ skipped`, so this shows as passed — the warning is + // the signal. A *static* force-gate never reaches here; it is a real skip. + require('console').warn( + ` ⚠ skipped by \`@force-gate ${decision.gate.source}\`` + ) + return + } + + const error = new Error(staleGateMessage(decision.gate)) + Error.captureStackTrace(error, body) + await expectTestToFail(() => callback.call(this), error) + require('console').warn( + ` ⚠ gated test failed as expected (@gate ${decision.gate.source})` + ) + } + + gatedBodies.add(body) + return body as jest.ProvidesCallback +} + +/** + * The body for a test registered through Jest's own `test.failing`, used when + * a false static `@gate` is known at collection time. Jest inverts the outcome + * natively; this wrapper only keeps the log lines consistent with the + * runtime-inverted (lazy) path. + */ +function wrapFailingBody( + gate: Gate, + callback: Function +): jest.ProvidesCallback { + if (callback.length > 0) { + throw new Error( + `A gated test cannot use the \`done\` callback, because the gate has to ` + + `observe whether the test failed. Return a promise instead.` + ) + } + + const body = async function gatedFailingBody(this: unknown): Promise { + try { + await callback.call(this) + } catch (error) { + require('console').warn( + ` ⚠ gated test failed as expected (@gate ${gate.source})` + ) + throw error + } + // The body passed. Jest is about to fail this test with its generic + // "Failing test passed even though it was supposed to fail" error, which + // points at a `.failing` modifier the author never wrote — so explain the + // real situation alongside it. + require('console').warn(staleGateMessage(gate)) + } + + gatedBodies.add(body) + return body as jest.ProvidesCallback +} + +/** + * Finds a false static `@gate` at collection time, which decides the whole + * test early: it registers through Jest's native `test.failing` instead of + * wrapping the body. Not applicable when a lazy `@force-gate` is also present + * — that could override the inversion with a force-pass, so the decision has + * to wait for the fixture's config at runtime. (If a lazy plain gate is false + * as well, the outcome is an inversion either way; the static one is simply + * the gate that gets named.) + */ +function findStaticInversion(gates: Gate[]): Gate | null { + if (gates.some((gate) => gate.force && gate.needsResolvedConfig)) { + return null + } + return ( + gates.find( + (gate) => + !gate.force && + !gate.needsResolvedConfig && + !evaluate(gate.node, (name) => readCondition(name)) + ) ?? null + ) +} + +function resolveTestFn(kind: string): TestFn { + const g = global as any + switch (kind) { + case 'it': + return g.it + case 'test': + return g.test + case 'fit': + return g.fit ?? g.it.only + case 'it.only': + return g.it.only + case 'test.only': + return g.test.only + case 'describe': + return g.describe + case 'describe.only': + return g.describe.only + default: + throw new Error(`\`@gate\` does not support \`${kind}(...)\``) + } +} + +/** The `.skip` counterpart of `resolveTestFn`, for a false `@force-gate`. */ +function resolveSkipFn(kind: string): TestFn { + const g = global as any + if (kind.startsWith('describe')) return g.describe.skip + if (kind.startsWith('test')) return g.test.skip + return g.it.skip +} + +export function _test_gate(pragmas: GatePragma[], kind: string) { + // Parsing and validation happen while the test file is being collected, so a + // typo'd condition fails the whole suite instead of one test. + const allGates = pragmas.map(parseGate) + // A static `@force-gate` is decided at collection time (a real Jest skip). + // Everything else — `@gate`, and *lazy* `@force-gate` — is resolved at + // runtime, so it inherits down into the tests via the describe stack. + const staticForceGates = allGates.filter( + (gate) => gate.force && !gate.needsResolvedConfig + ) + const runtimeGates = allGates.filter( + (gate) => !gate.force || gate.needsResolvedConfig + ) + const isDescribe = kind.startsWith('describe') + const testFn = resolveTestFn(kind) + + return function gated(name: string, callback: Function, timeout?: number) { + // A false static `@force-gate` is a real Jest skip, decided right here. + const forcedOff = staticForceGates.find( + (gate) => !evaluate(gate.node, (condition) => readCondition(condition)) + ) + if (forcedOff) { + return resolveSkipFn(kind)( + name, + callback as jest.ProvidesCallback, + timeout + ) + } + + if (isDescribe) { + // Register the `describe` normally, but make its runtime gates (including + // a lazy `@force-gate`) visible while its body is collected so nested + // tests inherit them and `nextTestSetup` can gate the build. + return testFn(name, function (this: unknown) { + describeGateStack.push(...runtimeGates) + try { + return callback.call(this) + } finally { + describeGateStack.length -= runtimeGates.length + } + } as jest.ProvidesCallback) + } + + const applicable = [...describeGateStack, ...runtimeGates] + + const staticInversion = findStaticInversion(applicable) + const failingFn = (testFn as { failing?: TestFn }).failing + if (staticInversion && typeof failingFn === 'function') { + return failingFn( + name, + wrapFailingBody(staticInversion, callback), + timeout + ) + } + + return testFn(name, wrapGatedBody(applicable, callback), timeout) + } +} + +/** + * A snapshot of the gates on the enclosing `describe`(s), taken while the + * describe body is being collected. `nextTestSetup` reads this synchronously to + * find a lazy `@force-gate` that should gate the fixture build. The stack is + * empty again once collection finishes, so it must be read at call time. + */ +export function getActiveDescribeGates(): Gate[] { + return [...describeGateStack] +} + +/** Whether any of `gates` is a lazy `@force-gate` (resolved from config). */ +export function hasLazyForceGate(gates: Gate[]): boolean { + return gates.some((gate) => gate.force && gate.needsResolvedConfig) +} + +/** + * The first lazy `@force-gate` in `gates` whose condition is false against the + * resolved `config` — i.e. the one that says "don't build this fixture here" — + * or `null` if none apply. Used by `nextTestSetup` to skip the build. + */ +export function findLazyForceSkip( + gates: Gate[], + config: ResolvedNextConfig +): GatePragma | null { + for (const gate of gates) { + if ( + gate.force && + gate.needsResolvedConfig && + !evaluate(gate.node, (name) => readCondition(name, config)) + ) { + return gate + } + } + return null +} + +/** + * Every `it` / `test` — gated or not — has to consult the enclosing + * `describe`'s gates, so the globals are wrapped once. This is the same + * technique `test/lib/e2e-utils` uses to inject a per-test timeout, and the two + * wrappers compose. + * + * Known gap: `it.each` and friends bypass the wrapper, so a `describe`-level + * gate does not reach them. `it.each` cannot carry a pragma of its own either + * (the transform rejects it). + */ +function wrapTestGlobals(): void { + for (const key of ['it', 'test'] as const) { + const original = (global as any)[key] + if (typeof original !== 'function' || original.__gateWrapped) continue + + const wrapped = new Proxy(original, { + apply(target, thisArg, args: any[]) { + const [name, callback, timeout] = args + if ( + describeGateStack.length === 0 || + typeof callback !== 'function' || + gatedBodies.has(callback) + ) { + return Reflect.apply(target, thisArg, args) + } + + const applicable = [...describeGateStack] + const staticInversion = findStaticInversion(applicable) + const failingFn = (target as { failing?: TestFn }).failing + if (staticInversion && typeof failingFn === 'function') { + return Reflect.apply(failingFn, thisArg, [ + name, + wrapFailingBody(staticInversion, callback), + timeout, + ]) + } + + return Reflect.apply(target, thisArg, [ + name, + wrapGatedBody(applicable, callback), + timeout, + ]) + }, + }) + Object.defineProperty(wrapped, '__gateWrapped', { value: true }) + ;(global as any)[key] = wrapped + } +} + +/** Called from `test/jest-setup-after-env.ts`. */ +export function installGate(): void { + ;(global as any)._test_gate = _test_gate + wrapTestGlobals() +} + +/** Test-only: the parse/validate half, without registering anything. */ +export const __testing = { parseGate, decideGates, wrapGatedBody } diff --git a/test/lib/gate/state.ts b/test/lib/gate/state.ts new file mode 100644 index 000000000000..034e1f6317e4 --- /dev/null +++ b/test/lib/gate/state.ts @@ -0,0 +1,51 @@ +/** + * The seam between the gate runtime and the Next.js fixture a suite booted. + * + * A fixture is registered by `createNext()` as soon as it is set up, and + * unregistered when it is destroyed. Registering the *instance* rather than a + * config snapshot is what keeps `skipStart: true` suites and `next.build()` / + * restart flows working: nothing is resolved until a gate actually asks, by + * which point the fixture is up. + * + * Suites with no lazy `@gate` therefore pay nothing at all. + */ + +import type { ResolvedNextConfig } from './resolved-config' + +/** + * The part of `NextInstance` the gate runtime needs. Structural, so this module + * stays independent of `test/lib/next-modes/base.ts`. + */ +export type GateConfigSource = { + getResolvedConfig(): Promise +} + +let activeFixture: GateConfigSource | null = null + +export function registerFixture(fixture: GateConfigSource): void { + activeFixture = fixture +} + +export function clearFixture(): void { + activeFixture = null +} + +export function hasFixture(): boolean { + return activeFixture !== null +} + +/** + * Resolves the running fixture's config for a lazy condition. Memoization lives + * on the instance, so repeated gates in one file resolve it once. + */ +export function getResolvedConfigForGates(): Promise { + if (!activeFixture) { + throw new Error( + `This \`@gate\` condition is resolved from the running Next.js ` + + `fixture's config, but no fixture is registered. Is this suite using ` + + `\`nextTestSetup()\`? Conditions that are known up front (\`dev\`, ` + + `\`turbopack\`, ...) do not need one.` + ) + } + return activeFixture.getResolvedConfig() +} diff --git a/test/lib/gate/test-context.ts b/test/lib/gate/test-context.ts new file mode 100644 index 000000000000..5dc6b61db36a --- /dev/null +++ b/test/lib/gate/test-context.ts @@ -0,0 +1,46 @@ +/** + * The statically-known shape of the current test run, published by `e2e-utils` + * when it is imported and read by the static conditions in `./conditions.ts`. + * + * `e2e-utils` derives all of this at module scope (from `NEXT_TEST_MODE`, the + * test's folder, and the bundler env vars), and a test file imports `e2e-utils` + * before any `_test_gate(...)` call runs, so the context is always populated by + * the time a gate is evaluated in an e2e suite. + * + * It lives in its own module so that the gate runtime — which is loaded for + * every Jest project, including unit tests — never has to import `e2e-utils` + * and its side effects. + */ + +export type GateTestMode = 'dev' | 'start' | 'deploy' +export type GateTestBundler = 'turbopack' | 'rspack' | 'webpack' + +export type GateTestContext = { + mode: GateTestMode + bundler: GateTestBundler + react18: boolean + wasm: boolean +} + +let current: GateTestContext | null = null + +export function setGateTestContext(context: GateTestContext): void { + current = context +} + +export function getGateTestContext(): GateTestContext { + if (!current) { + throw new Error( + `This \`@gate\` condition describes the e2e test run (mode, bundler, ` + + `React version), but no run context has been recorded. Conditions ` + + `like \`dev\` and \`turbopack\` are only available to suites that ` + + `import \`e2e-utils\`.` + ) + } + return current +} + +/** Test-only. */ +export function clearGateTestContext(): void { + current = null +} diff --git a/test/lib/next-modes/base.ts b/test/lib/next-modes/base.ts index 7b813903d2e7..233b94a6a495 100644 --- a/test/lib/next-modes/base.ts +++ b/test/lib/next-modes/base.ts @@ -22,6 +22,9 @@ import type { Playwright } from '../browsers/playwright' import escapeStringRegexp from 'escape-string-regexp' import * as JSON5 from 'json5' import { Page, Response } from 'playwright' +import { PHASE_PRODUCTION_BUILD } from 'next/constants' +import { loadResolvedConfig } from '../gate/load-resolved-config' +import type { ResolvedNextConfig } from '../gate/resolved-config' type Event = 'stdout' | 'stderr' | 'error' | 'destroy' export type InstallCommand = @@ -111,6 +114,7 @@ export class NextInstance { public serverReadyPattern: RegExp = /✓ Ready in / patchFileDelay: number = 0 public deleteWorkspaceFile: boolean = false + private _resolvedConfig?: Promise constructor(opts: NextInstanceOpts) { this.env = {} @@ -531,6 +535,60 @@ export class NextInstance { }) } + /** + * The phase this fixture's `next.config` should be resolved for. `next dev` + * overrides it; deploy mode resolves the production-build phase locally, + * which is a best-effort approximation of the remote build. + */ + protected get configPhase(): string { + return PHASE_PRODUCTION_BUILD + } + + /** + * This fixture's **resolved** `next.config` — i.e. the output of + * `loadConfig`, not the config file. Resolution implies flags the fixture + * never mentions (`cacheComponents: true` turns on `experimental.ppr`) and + * honours the env the fixture actually runs with. + * + * Used by `// @gate` conditions (see test/lib/gate/README.md). Memoized per + * instance and resolved lazily, so a suite that never asks pays nothing; a + * suite that rewrites its own `next.config` mid-run keeps the first answer. + */ + public getResolvedConfig(): Promise { + return (this._resolvedConfig ??= loadResolvedConfig({ + dir: this.testDir, + phase: this.configPhase, + env: this.getSpawnOpts().env, + })) + } + + /** + * The options every child process of this fixture is spawned with — its cwd, + * and the exact env `next build` / `next dev` / `next start` see. + * + * Anything that inspects the fixture out of process (e.g. resolving its + * `next.config`) has to use this env, because config resolution reads env + * vars from the calling process and a suite may pass its own via + * `nextTestSetup({ env })`. + */ + protected getSpawnOpts( + env?: Record + ): import('child_process').SpawnOptions { + return { + cwd: this.testDir, + stdio: ['ignore', 'pipe', 'pipe'], + shell: false, + env: { + ...process.env, + ...this.env, + ...env, + NODE_ENV: this.env.NODE_ENV || ('' as any), + PORT: this.forcedPort ?? '0', + __NEXT_TEST_MODE: 'e2e', + }, + } + } + protected setServerReadyTimeout( reject: (reason?: unknown) => void, ms: number diff --git a/test/lib/next-modes/next-dev.ts b/test/lib/next-modes/next-dev.ts index 95906d354ffa..2e3935b14329 100644 --- a/test/lib/next-modes/next-dev.ts +++ b/test/lib/next-modes/next-dev.ts @@ -1,6 +1,7 @@ import spawn from 'cross-spawn' import { Span } from 'next/dist/trace' import { NextInstance } from './base' +import { PHASE_DEVELOPMENT_SERVER } from 'next/constants' import { retry, waitFor } from 'next-test-utils' import stripAnsi from 'strip-ansi' import { quote as shellQuote } from 'shell-quote' @@ -12,6 +13,10 @@ export class NextDevInstance extends NextInstance { return 'development' } + protected get configPhase() { + return PHASE_DEVELOPMENT_SERVER + } + public async setup(parentSpan: Span) { super.setup(parentSpan) await super.createTestDir({ parentSpan }) @@ -61,24 +66,6 @@ export class NextDevInstance extends NextInstance { return buildArgs } - private getSpawnOpts( - env?: Record - ): import('child_process').SpawnOptions { - return { - cwd: this.testDir, - stdio: ['ignore', 'pipe', 'pipe'], - shell: false, - env: { - ...process.env, - ...this.env, - ...env, - NODE_ENV: this.env.NODE_ENV || ('' as any), - PORT: this.forcedPort || '0', - __NEXT_TEST_MODE: 'e2e', - }, - } - } - public async build( options: { env?: Record; args?: string[] } = {} ) { diff --git a/test/lib/next-modes/next-start.ts b/test/lib/next-modes/next-start.ts index 553a38fe49d8..21286c7f8274 100644 --- a/test/lib/next-modes/next-start.ts +++ b/test/lib/next-modes/next-start.ts @@ -266,24 +266,6 @@ export class NextStartInstance extends NextInstance { return buildArgs } - private getSpawnOpts( - env?: Record - ): import('child_process').SpawnOptions { - return { - cwd: this.testDir, - stdio: ['ignore', 'pipe', 'pipe'], - shell: false, - env: { - ...process.env, - ...this.env, - ...env, - NODE_ENV: this.env.NODE_ENV || ('' as any), - PORT: this.forcedPort ?? '0', - __NEXT_TEST_MODE: 'e2e', - }, - } - } - public async build( options: { env?: Record; args?: string[] } = {} ) { diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index e4a679a7ac8a..e14d4634c26b 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -43,6 +43,9 @@ import { RequiredServerFilesManifest } from 'next/dist/build' export { shouldUseTurbopack } +// The runtime counterpart of the `// @gate` pragma. See test/lib/gate/README.md. +export { gate } from './gate/runtime' + export const nextServer = server export const pkg = _pkg diff --git a/test/production/next-server-nft/next-server-nft.test.ts b/test/production/next-server-nft/next-server-nft.test.ts index 4ad2a0e0f185..6a914dba10eb 100644 --- a/test/production/next-server-nft/next-server-nft.test.ts +++ b/test/production/next-server-nft/next-server-nft.test.ts @@ -707,7 +707,6 @@ async function readNormalizedNFT(next, name) { "/node_modules/next/dist/server/route-modules/app-page/vendored/contexts/router-context.js", "/node_modules/next/dist/server/route-modules/app-page/vendored/contexts/server-inserted-html.js", "/node_modules/next/dist/server/runtime-reacts.external.js", - "/node_modules/next/dist/server/web/spec-extension/adapters/reflect.js", "/node_modules/next/dist/shared/lib/deep-freeze.js", "/node_modules/next/dist/shared/lib/instant-messages.js", "/node_modules/next/dist/shared/lib/invariant-error.js", diff --git a/test/unit/gate/expr.test.ts b/test/unit/gate/expr.test.ts new file mode 100644 index 000000000000..4602fe00bbcb --- /dev/null +++ b/test/unit/gate/expr.test.ts @@ -0,0 +1,59 @@ +import { evaluate, parse } from '../../lib/gate/expr' + +const run = (source: string, values: Record = {}) => + evaluate(parse(source).node, (name) => values[name]) + +describe('@gate expression language', () => { + it('reads a bare condition by truthiness', () => { + expect(run('a', { a: true })).toBe(true) + expect(run('a', { a: false })).toBe(false) + expect(run('a', {})).toBe(false) + // A condition whose resolved value is an object (e.g. prefetchInlining). + expect(run('a', { a: { maxSize: 2048 } })).toBe(true) + }) + + it('supports negation', () => { + expect(run('!a', { a: false })).toBe(true) + expect(run('!!a', { a: false })).toBe(false) + }) + + it('supports && and ||', () => { + expect(run('a && b', { a: true, b: true })).toBe(true) + expect(run('a && b', { a: true, b: false })).toBe(false) + expect(run('a || b', { a: false, b: true })).toBe(true) + expect(run('!a && b', { a: false, b: true })).toBe(true) + }) + + it('supports parentheses', () => { + expect(run('!(a && b)', { a: true, b: false })).toBe(true) + expect(run('!a && (b || c)', { a: false, b: false, c: true })).toBe(true) + }) + + it('compares against string literals', () => { + expect(run("mode === 'start'", { mode: 'start' })).toBe(true) + expect(run("mode === 'start'", { mode: 'dev' })).toBe(false) + expect(run('mode !== "dev"', { mode: 'start' })).toBe(true) + expect(run("mode == 'dev'", { mode: 'dev' })).toBe(true) + }) + + it('compares against booleans', () => { + expect(run('a === false', { a: false })).toBe(true) + expect(run('a === true', { a: undefined })).toBe(false) + }) + + it('collects the referenced condition names, deduplicated', () => { + expect(parse("!a && (b || a) && c === 'x'").names).toEqual(['a', 'b', 'c']) + expect(parse("mode === 'dev'").names).toEqual(['mode']) + }) + + it.each([ + ['', 'Unexpected end of expression'], + ['a &&', 'Unexpected end of expression'], + ['(a', 'Missing closing `)`'], + ['a b', 'after a complete expression'], + ["'unterminated", 'Unterminated string'], + ['a # b', 'Unexpected character'], + ])('reports a syntax error for %p', (source, message) => { + expect(() => parse(source)).toThrow(message) + }) +}) diff --git a/test/unit/gate/pragma-transform.test.ts b/test/unit/gate/pragma-transform.test.ts new file mode 100644 index 000000000000..1f9c81e694ce --- /dev/null +++ b/test/unit/gate/pragma-transform.test.ts @@ -0,0 +1,184 @@ +/* eslint-env jest */ +const { rewrite } = require('../../lib/gate/pragma-transform') + +// NOTE: fixtures are built by joining arrays of lines instead of using template +// literals. A template literal containing a `// @gate` line directly above an +// `it(` line would itself be rewritten when *this* file is transformed, since +// the rewrite is intentionally a line-oriented regex over the raw source. +const src = (...lines: string[]) => lines.join('\n') + '\n' + +describe('@gate pragma transform', () => { + it('leaves files without a pragma untouched (identity, not a copy)', () => { + const input = src("it('a', () => {})") + expect(rewrite(input, 'x.test.ts')).toBe(input) + }) + + it('rewrites a single pragma over `it`', () => { + const out = rewrite( + src('// @gate !cacheComponents', "it('a', () => {})"), + 'x.test.ts' + ) + expect(out).toBe( + src( + '// @gate !cacheComponents', + `_test_gate([{"force":false,"source":"!cacheComponents"}],"it")('a', () => {})` + ) + ) + }) + + it('preserves indentation and line count', () => { + const input = src( + 'describe(() => {', + ' // @gate dev', + " it('a', () => {", + ' expect(1).toBe(1)', + ' })', + '})' + ) + const out = rewrite(input, 'x.test.ts') + expect(out.split('\n')).toHaveLength(input.split('\n').length) + expect(out.split('\n')[2]).toBe( + ` _test_gate([{"force":false,"source":"dev"}],"it")('a', () => {` + ) + // Every line other than the rewritten call site is byte-identical. + const inLines = input.split('\n') + const outLines = out.split('\n') + for (let i = 0; i < inLines.length; i++) { + if (i === 2) continue + expect(outLines[i]).toBe(inLines[i]) + } + }) + + it('combines consecutive pragmas', () => { + const out = rewrite( + src('// @gate dev', '// @force-gate turbopack', "test('a', () => {})"), + 'x.test.ts' + ) + expect(out.split('\n')[2]).toBe( + `_test_gate([{"force":false,"source":"dev"},{"force":true,"source":"turbopack"}],"test")('a', () => {})` + ) + }) + + it('rewrites a `@force-gate`-only file (no `@gate` substring)', () => { + // Guards the cheap bail-out: `@force-gate` does not contain `@gate`. + const input = src( + '// @force-gate !cacheComponents', + "describe('s', () => {})" + ) + expect(input.includes('@gate')).toBe(false) + const out = rewrite(input, 'x.test.ts') + expect(out.split('\n')[1]).toBe( + `_test_gate([{"force":true,"source":"!cacheComponents"}],"describe")('s', () => {})` + ) + }) + + it.each([ + ['it', 'it'], + ['test', 'test'], + ['fit', 'fit'], + ['describe', 'describe'], + ['it.only', 'it.only'], + ['test.only', 'test.only'], + ['describe.only', 'describe.only'], + ])('supports %s', (callee, kind) => { + const out = rewrite(src('// @gate dev', `${callee}('a', () => {})`), 'x.ts') + expect(out.split('\n')[1]).toBe( + `_test_gate([{"force":false,"source":"dev"}],${JSON.stringify( + kind + )})('a', () => {})` + ) + }) + + it('keeps the condition source verbatim, including quotes', () => { + const out = rewrite( + src("// @gate mode === 'start' && !turbopack", "it('a', () => {})"), + 'x.test.ts' + ) + expect(out.split('\n')[1]).toBe( + `_test_gate([{"force":false,"source":"mode === 'start' && !turbopack"}],"it")('a', () => {})` + ) + }) + + describe('inert pragmas are hard errors', () => { + it('rejects a pragma separated from the call by a blank line', () => { + expect(() => + rewrite(src('// @gate dev', '', "it('a', () => {})"), 'x.test.ts') + ).toThrow('this `@gate` pragma has no effect') + }) + + it('rejects a pragma inside a JSDoc block', () => { + expect(() => + rewrite( + src('/**', ' * some docs', ' */', '// @gate dev', 'const x = 1'), + 'x.test.ts' + ) + ).toThrow('this `@gate` pragma has no effect') + }) + + it('rejects a pragma over it.each', () => { + expect(() => + rewrite(src('// @gate dev', 'it.each([1])("a", () => {})'), 'x.test.ts') + ).toThrow('this `@gate` pragma has no effect') + }) + + it('rejects a pragma over it.skip as ambiguous', () => { + expect(() => + rewrite(src('// @gate dev', "it.skip('a', () => {})"), 'x.test.ts') + ).toThrow('a `@gate` pragma on a skipped test is ambiguous') + }) + + it('rejects a pragma over xit / xdescribe / it.todo as ambiguous', () => { + for (const call of [ + "xit('a', () => {})", + "xdescribe('a', () => {})", + "it.todo('a')", + "describe.skip('a', () => {})", + ]) { + expect(() => rewrite(src('// @gate dev', call), 'x.test.ts')).toThrow( + 'a `@gate` pragma on a skipped test is ambiguous' + ) + } + }) + + it('rejects a stacked pragma block over a skip, pointing at the first pragma', () => { + expect(() => + rewrite( + src('// @force-gate !dev', '// @gate dev', "xit('a', () => {})"), + 'x.test.ts' + ) + ).toThrow(/x\.test\.ts:1[\s\S]*ambiguous/) + }) + + it('leaves a skip without a pragma alone', () => { + const input = src( + '// @gate dev', + "it('gated', () => {})", + "xit('skipped', () => {})", + "it.skip('also skipped', () => {})" + ) + const out = rewrite(input, 'x.test.ts') + expect(out).toContain("xit('skipped', () => {})") + expect(out).toContain("it.skip('also skipped', () => {})") + }) + + it('reports the file and line', () => { + expect(() => + rewrite(src('const x = 1', '// @gate dev', 'const y = 2'), 'x.test.ts') + ).toThrow('x.test.ts:2') + }) + + it('rejects a pragma with no condition', () => { + expect(() => + rewrite(src('// @gate', "it('a', () => {})"), 'x.test.ts') + ).toThrow('needs a condition') + }) + }) + + it('does not match prose comments that merely mention the word', () => { + const input = src( + '// Use @gate to mark a known failure.', + "it('a', () => {})" + ) + expect(rewrite(input, 'x.test.ts')).toBe(input) + }) +}) diff --git a/test/unit/gate/runtime.test.ts b/test/unit/gate/runtime.test.ts new file mode 100644 index 000000000000..63aeeb85ac6e --- /dev/null +++ b/test/unit/gate/runtime.test.ts @@ -0,0 +1,418 @@ +// Imported through the same specifier tests use, so this also covers the +// re-export from next-test-utils. +import { gate } from 'next-test-utils' +import { __testing, _test_gate, expectTestToFail } from '../../lib/gate/runtime' +import { + clearGateTestContext, + setGateTestContext, +} from '../../lib/gate/test-context' +import { clearFixture, registerFixture } from '../../lib/gate/state' + +// A false *static* `@gate` is decided at collection time (it registers +// through Jest's native `test.failing`), and collection runs before any +// `beforeEach` — so the pretend run context has to be pinned at module scope +// as well. The `beforeEach` below re-pins it for each test body. +setGateTestContext({ + mode: 'start', + bundler: 'webpack', + react18: false, + wasm: false, +}) + +const parseGate = (source: string, force = false) => + __testing.parseGate({ force, source }) + +/** Builds the wrapped body the transform would have installed, and runs it. */ +const runGated = (sources: string[], body: () => unknown) => { + const wrapped = __testing.wrapGatedBody( + sources.map((s) => parseGate(s)), + body + ) + return (wrapped as () => Promise)() +} + +type FakeTestFn = jest.Mock & { + skip: jest.Mock + only: jest.Mock & { failing: jest.Mock } + failing: jest.Mock +} + +const makeFakeTestFn = (): FakeTestFn => + Object.assign(jest.fn(), { + skip: jest.fn(), + only: Object.assign(jest.fn(), { failing: jest.fn() }), + failing: jest.fn(), + }) as FakeTestFn + +/** + * Swaps `global.it` / `global.test` / `global.describe` for spies while `fn` + * runs, so `_test_gate`'s registration decisions can be observed without + * actually registering tests. + */ +const withFakeTestGlobals = (fn: () => void) => { + const fakes = { + it: makeFakeTestFn(), + test: makeFakeTestFn(), + describe: makeFakeTestFn(), + } + const originals = { + it: global.it, + test: global.test, + describe: global.describe, + } + Object.assign(global, fakes) + try { + fn() + } finally { + Object.assign(global, originals) + } + return fakes +} + +const fixtureWith = (config: Record) => { + const getResolvedConfig = jest.fn(async () => config) + registerFixture({ getResolvedConfig }) + return getResolvedConfig +} + +describe('@gate runtime', () => { + let warn: jest.SpyInstance + + beforeEach(() => { + setGateTestContext({ + mode: 'start', + bundler: 'webpack', + react18: false, + wasm: false, + }) + warn = jest.spyOn(require('console'), 'warn').mockImplementation(() => {}) + }) + + afterEach(() => { + warn.mockRestore() + clearFixture() + clearGateTestContext() + }) + + describe('a gate that holds', () => { + it('runs the body and lets it pass', async () => { + const body = jest.fn() + await runGated(['!dev'], body) + expect(body).toHaveBeenCalled() + expect(warn).not.toHaveBeenCalled() + }) + + it('lets a failure through', async () => { + await expect( + runGated(['!dev'], () => { + throw new Error('boom') + }) + ).rejects.toThrow('boom') + }) + }) + + describe('a gate that is false', () => { + it('absorbs a failing body and reports it', async () => { + await runGated(['dev'], () => { + throw new Error('boom') + }) + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('gated test failed as expected (@gate dev)') + ) + }) + + it('absorbs a rejected promise', async () => { + await runGated(['dev'], async () => { + throw new Error('boom') + }) + expect(warn).toHaveBeenCalled() + }) + + it('FAILS when the body passes, naming the stale pragma', async () => { + await expect(runGated(['dev'], () => {})).rejects.toThrow( + /Gated test passed unexpectedly[\s\S]*The gate is stale: delete the `\/\/ @gate dev` pragma/ + ) + }) + + it('reports the first false gate when several are applied', async () => { + await expect(runGated(['!start', 'dev'], () => {})).rejects.toThrow( + '`// @gate !start` pragma' + ) + }) + }) + + describe('lazy conditions', () => { + it('reads the running fixture’s resolved config', async () => { + const getResolvedConfig = fixtureWith({ cacheComponents: true }) + // cacheComponents is on, so `!cacheComponents` is false: a passing body + // is a stale gate. + await expect(runGated(['!cacheComponents'], () => {})).rejects.toThrow( + 'Gated test passed unexpectedly' + ) + expect(getResolvedConfig).toHaveBeenCalled() + }) + + it('holds when the config says so', async () => { + fixtureWith({ cacheComponents: false }) + const body = jest.fn() + await runGated(['!cacheComponents'], body) + expect(body).toHaveBeenCalled() + }) + + it('reads keys that stayed under `experimental`', async () => { + fixtureWith({ cacheComponents: true, experimental: { ppr: true } }) + const body = jest.fn() + await runGated(['ppr && cacheComponents'], body) + expect(body).toHaveBeenCalled() + }) + + it('is not resolved at all for a static-only gate', async () => { + const getResolvedConfig = fixtureWith({ cacheComponents: true }) + await runGated(['!dev'], () => {}) + expect(getResolvedConfig).not.toHaveBeenCalled() + }) + + it('explains itself when no fixture is registered', async () => { + clearFixture() + await expect(runGated(['cacheComponents'], () => {})).rejects.toThrow( + 'no fixture is registered' + ) + }) + }) + + describe('the runtime gate() import', () => { + it('answers a predicate over the conditions, without inverting anything', async () => { + expect(await gate((c) => !c.dev)).toBe(true) + expect(await gate((c) => c.dev)).toBe(false) + expect(await gate((c) => c.mode === 'start' && c.webpack)).toBe(true) + }) + + it('reads a lazy condition from the running fixture', async () => { + const getResolvedConfig = fixtureWith({ cacheComponents: true }) + expect(await gate((c) => c.cacheComponents)).toBe(true) + expect(await gate((c) => !c.cacheComponents)).toBe(false) + expect(getResolvedConfig).toHaveBeenCalled() + }) + + it('rejects an undeclared condition like the pragma does', async () => { + await expect(gate((c) => c.cacheComponnents)).rejects.toThrow( + 'references an undeclared condition "cacheComponnents"' + ) + }) + + it('supports static predicates in suites with no fixture', async () => { + clearFixture() + expect(await gate((c) => c.start)).toBe(true) + await expect(gate((c) => c.cacheComponents)).rejects.toThrow( + 'no fixture is registered' + ) + }) + + it('also accepts the pragma expression language as a string', async () => { + fixtureWith({ cacheComponents: true }) + expect(await gate("mode === 'start' && webpack")).toBe(true) + expect(await gate('!cacheComponents')).toBe(false) + await expect(gate('cacheComponnents')).rejects.toThrow( + 'references an undeclared condition "cacheComponnents"' + ) + }) + + it('does not resolve the config for a static-only string expression', async () => { + const getResolvedConfig = fixtureWith({ cacheComponents: true }) + await gate('!dev') + expect(getResolvedConfig).not.toHaveBeenCalled() + }) + }) + + describe('collection-time validation', () => { + it('rejects an undeclared condition and lists the declared ones', () => { + expect(() => parseGate('cacheComponnents')).toThrow( + 'references an undeclared condition "cacheComponnents"' + ) + expect(() => parseGate('cacheComponnents')).toThrow( + /Declared conditions: .*cacheComponents/ + ) + }) + + it('rejects an unparsable expression, quoting the pragma', () => { + expect(() => parseGate('dev &&')).toThrow( + 'Could not parse `// @gate dev &&`' + ) + }) + + it('rejects a `done`-callback test', () => { + expect(() => + __testing.wrapGatedBody([parseGate('dev')], (done: unknown) => done) + ).toThrow('cannot use the `done` callback') + }) + }) + + describe('@force-gate', () => { + it('accepts a lazy condition, classified for runtime resolution', () => { + const gate = parseGate('!cacheComponents', true) + expect(gate.force).toBe(true) + expect(gate.needsResolvedConfig).toBe(true) + }) + + it('accepts a static condition, decided at collection', () => { + const gate = parseGate('!dev', true) + expect(gate.force).toBe(true) + expect(gate.needsResolvedConfig).toBe(false) + }) + + it('skips the test for real when the condition is false', () => { + const body = () => {} + const fakes = withFakeTestGlobals(() => { + _test_gate([{ force: true, source: 'dev' }], 'it')('a test', body) + }) + expect(fakes.it.skip).toHaveBeenCalledWith('a test', body, undefined) + expect(fakes.it).not.toHaveBeenCalled() + }) + + it('registers the test normally when the condition holds', () => { + const fakes = withFakeTestGlobals(() => { + _test_gate([{ force: true, source: '!dev' }], 'it')('a test', () => {}) + }) + expect(fakes.it).toHaveBeenCalledTimes(1) + expect(fakes.it.skip).not.toHaveBeenCalled() + }) + + it('skips a whole describe with describe.skip', () => { + const fakes = withFakeTestGlobals(() => { + _test_gate([{ force: true, source: 'dev' }], 'describe')( + 'a suite', + () => {} + ) + }) + expect(fakes.describe.skip).toHaveBeenCalledWith( + 'a suite', + expect.any(Function), + undefined + ) + expect(fakes.describe).not.toHaveBeenCalled() + }) + + it('leaves a `@gate` on the same test in charge when it holds', async () => { + const fakes = withFakeTestGlobals(() => { + _test_gate( + [ + { force: true, source: '!dev' }, + { force: false, source: 'dev' }, + ], + 'it' + )('a test', () => {}) + }) + expect(fakes.it.skip).not.toHaveBeenCalled() + // The non-force gate is static and false, so the test registers through + // Jest's native `test.failing`. Jest inverts the outcome itself; the + // wrapper only logs. A passing body warns that the gate is stale (Jest + // then fails the test on its own). + expect(fakes.it).not.toHaveBeenCalled() + expect(fakes.it.failing).toHaveBeenCalledTimes(1) + const registered = fakes.it.failing.mock + .calls[0][1] as () => Promise + await registered() + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('Gated test passed unexpectedly') + ) + }) + + it('logs when a `test.failing` body fails as expected', async () => { + const fakes = withFakeTestGlobals(() => { + _test_gate([{ force: false, source: 'dev' }], 'it')('a test', () => { + throw new Error('boom') + }) + }) + const registered = fakes.it.failing.mock + .calls[0][1] as () => Promise + await expect(registered()).rejects.toThrow('boom') + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('gated test failed as expected (@gate dev)') + ) + }) + + it('keeps the decision at runtime when a lazy force-gate could override it', () => { + const fakes = withFakeTestGlobals(() => { + _test_gate( + [ + { force: true, source: '!cacheComponents' }, + { force: false, source: 'dev' }, + ], + 'it' + )('a test', () => {}) + }) + // `dev` is false, but the lazy `@force-gate !cacheComponents` might + // force-pass the test instead, and that can't be known until the + // fixture's config resolves — so no collection-time `test.failing`. + expect(fakes.it.failing).not.toHaveBeenCalled() + expect(fakes.it).toHaveBeenCalledTimes(1) + }) + + it('force-passes a lazy force-gate whose condition is false', async () => { + fixtureWith({ cacheComponents: true }) // `!cacheComponents` is false + const body = jest.fn() + const wrapped = __testing.wrapGatedBody( + [parseGate('!cacheComponents', true)], + body + ) + await (wrapped as () => Promise)() + expect(body).not.toHaveBeenCalled() + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('skipped by `@force-gate !cacheComponents`') + ) + }) + + it('runs the body when a lazy force-gate condition holds', async () => { + fixtureWith({ cacheComponents: false }) // `!cacheComponents` is true + const body = jest.fn() + const wrapped = __testing.wrapGatedBody( + [parseGate('!cacheComponents', true)], + body + ) + await (wrapped as () => Promise)() + expect(body).toHaveBeenCalled() + }) + }) + + describe('a static condition outside the e2e harness', () => { + it('explains why it is unavailable', async () => { + clearGateTestContext() + await expect(runGated(['dev'], () => {})).rejects.toThrow( + 'no run context has been recorded' + ) + }) + }) + + // The tests above drive the runtime's internals directly. These two go + // through the real pragma path — the transform rewrote them, and the `it` / + // `describe` globals installed by `installGate()` registered them. This file + // pretends the run mode is `start`, so `dev` is false and a failing body is + // absorbed. The stale-gate direction cannot be asserted from inside Jest (the + // whole point is that it fails the test); see test/lib/gate/README.md. + // @gate dev + it('absorbs a failure through the real pragma path', () => { + expect(1).toBe(2) + }) + + // @gate dev + describe('a gate on a describe', () => { + it('is inherited by a test that has no pragma of its own', () => { + expect(1).toBe(2) + }) + }) + + describe('expectTestToFail', () => { + it('throws the provided error when the callback succeeds', async () => { + const error = new Error('should have failed') + await expect(expectTestToFail(() => {}, error)).rejects.toBe(error) + }) + + it('resolves when the callback throws', async () => { + await expect( + expectTestToFail(() => { + throw new Error('boom') + }, new Error('unused')) + ).resolves.toBeUndefined() + }) + }) +}) diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs index 07d374019628..588919fff858 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs @@ -551,13 +551,13 @@ impl TurboTasksBackend { done_event, )))) } - Some(InProgressState::InProgress(box InProgressStateInner { - done_event, .. - })) => Some(Ok(ReadOutcome::InProgress(listen_to_done_event( - reader_description, - tracking, - done_event, - )))), + Some(InProgressState::InProgress(InProgressStateInner { done_event, .. })) => { + Some(Ok(ReadOutcome::InProgress(listen_to_done_event( + reader_description, + tracking, + done_event, + )))) + } Some(InProgressState::Canceled) => Some(Err(anyhow::anyhow!( "{} was canceled", task.get_task_description() @@ -1869,7 +1869,7 @@ impl TurboTasksBackend { done_event, reason: _, } => done_event.notify(usize::MAX), - InProgressState::InProgress(box InProgressStateInner { done_event, .. }) => { + InProgressState::InProgress(InProgressStateInner { done_event, .. }) => { done_event.notify(usize::MAX) } InProgressState::Canceled => {} @@ -2210,7 +2210,7 @@ impl TurboTasksBackend { is_session_dependent, }); } - let &mut InProgressState::InProgress(box InProgressStateInner { + let &mut InProgressState::InProgress(InProgressStateInner { stale, ref mut new_children, once_task: is_once_task, @@ -2224,7 +2224,7 @@ impl TurboTasksBackend { #[cfg(not(feature = "no_fast_stale"))] if stale && !is_once_task { let stale_priority = compute_stale_priority(&task); - let Some(InProgressState::InProgress(box InProgressStateInner { + let Some(InProgressState::InProgress(InProgressStateInner { done_event, mut new_children, .. @@ -2609,7 +2609,7 @@ impl TurboTasksBackend { // Task was canceled in the meantime, so we don't connect the children return None; } - let InProgressState::InProgress(box InProgressStateInner { + let InProgressState::InProgress(InProgressStateInner { #[cfg(not(feature = "no_fast_stale"))] stale, once_task: is_once_task, @@ -2623,7 +2623,7 @@ impl TurboTasksBackend { #[cfg(not(feature = "no_fast_stale"))] if *stale && !is_once_task { let stale_priority = compute_stale_priority(&task); - let Some(InProgressState::InProgress(box InProgressStateInner { done_event, .. })) = + let Some(InProgressState::InProgress(InProgressStateInner { done_event, .. })) = task.take_in_progress() else { unreachable!(); @@ -2685,7 +2685,7 @@ impl TurboTasksBackend { // Task was canceled in the meantime, so we don't finish it return (None, None); } - let InProgressState::InProgress(box InProgressStateInner { + let InProgressState::InProgress(InProgressStateInner { done_event, once_task: is_once_task, stale, @@ -3250,7 +3250,7 @@ impl TurboTasksBackend { fn mark_own_task_as_finished(&self, task: TaskId, turbo_tasks: &TurboTasks) { let mut ctx = self.execute_context(turbo_tasks); let mut task = ctx.task(task, TaskDataCategory::Data); - if let Some(InProgressState::InProgress(box InProgressStateInner { + if let Some(InProgressState::InProgress(InProgressStateInner { marked_as_completed, .. })) = task.get_in_progress_mut() diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs index da56d19e9442..733de17d9e54 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/aggregation_update.rs @@ -1431,7 +1431,7 @@ impl AggregationUpdateQueue { self.inner_of_upper_lost_followers(ctx, lost_follower_ids, upper_id, retry); } } - AggregationUpdateJob::AggregatedDataUpdate(box AggregatedDataUpdateJob { + AggregationUpdateJob::AggregatedDataUpdate(AggregatedDataUpdateJob { upper_ids, update, }) => { diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/connect_child.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/connect_child.rs index 359c9e30cbaf..d098e6ac64f3 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/connect_child.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/connect_child.rs @@ -33,9 +33,8 @@ impl ConnectChildOperation { ) { if let Some(parent_task_id) = parent_task_id { let mut parent_task = ctx.task(parent_task_id, TaskDataCategory::Meta); - let Some(InProgressState::InProgress(box InProgressStateInner { - new_children, .. - })) = parent_task.get_in_progress() + let Some(InProgressState::InProgress(InProgressStateInner { new_children, .. })) = + parent_task.get_in_progress() else { panic!("Task is not in progress while calling another task: {parent_task:?}"); }; @@ -49,9 +48,8 @@ impl ConnectChildOperation { if parent_task.children_contains(&child_task_id) { // It is already connected, we can skip the rest // but we still need to update the new_children set - let Some(InProgressState::InProgress(box InProgressStateInner { - new_children, - .. + let Some(InProgressState::InProgress(InProgressStateInner { + new_children, .. })) = parent_task.get_in_progress_mut() else { unreachable!(); @@ -118,9 +116,8 @@ impl ConnectChildOperation { if let Some(parent_task_id) = parent_task_id { let mut parent_task = ctx.task(parent_task_id, TaskDataCategory::Meta); - let Some(InProgressState::InProgress(box InProgressStateInner { - new_children, .. - })) = parent_task.get_in_progress_mut() + let Some(InProgressState::InProgress(InProgressStateInner { new_children, .. })) = + parent_task.get_in_progress_mut() else { panic!("Task is not in progress while calling another task: {parent_task:?}"); }; diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs b/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs index 554d27edb44f..39908bf4a686 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/operation/invalidate.rs @@ -132,7 +132,7 @@ pub fn make_task_dirty_internal( #[cfg(feature = "trace_task_dirty")] let task_name = task.get_task_name(); if make_stale - && let Some(InProgressState::InProgress(box InProgressStateInner { stale, .. })) = + && let Some(InProgressState::InProgress(InProgressStateInner { stale, .. })) = task.get_in_progress_mut() && !*stale { diff --git a/turbopack/crates/turbo-tasks-backend/src/lib.rs b/turbopack/crates/turbo-tasks-backend/src/lib.rs index e7eececc584b..63b227c073fd 100644 --- a/turbopack/crates/turbo-tasks-backend/src/lib.rs +++ b/turbopack/crates/turbo-tasks-backend/src/lib.rs @@ -1,5 +1,5 @@ #![feature(anonymous_lifetime_in_impl_trait)] -#![feature(box_patterns)] +#![feature(deref_patterns)] mod backend; mod backing_storage; diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/eval_context.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/eval_context.rs index a386458d96a5..31d799f0c4b7 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/eval_context.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/eval_context.rs @@ -175,13 +175,13 @@ impl EvalContext { // Only treat literals as constant undefined, allowing arbitrary values inside here // would mean that they can have sideeffects, and `JsValue::Constant` can't model // that. - arg: box Expr::Lit(_), + arg: Expr::Lit(_), .. }) => JsValue::Constant(ConstantValue::Undefined), Expr::Unary(UnaryExpr { op: op!(unary, "-"), - arg: box Expr::Lit(Lit::Num(n)), + arg: Expr::Lit(Lit::Num(n)), .. }) => JsValue::Constant(ConstantValue::Num(ConstantNumber(-n.value))), @@ -288,9 +288,9 @@ impl EvalContext { }) => JsValue::r#in(arena, self.eval(arena, left), self.eval(arena, right)), &Expr::Cond(CondExpr { - box ref cons, - box ref alt, - box ref test, + ref cons, + ref alt, + ref test, .. }) => { let test = self.eval(arena, test); @@ -309,8 +309,8 @@ impl EvalContext { Expr::TaggedTpl(TaggedTpl { tag: - box Expr::Member(MemberExpr { - obj: box Expr::Ident(tag_obj), + Expr::Member(MemberExpr { + obj: Expr::Ident(tag_obj), prop: MemberProp::Ident(tag_prop), .. }), @@ -380,11 +380,7 @@ impl EvalContext { JsValue::member(arena, obj, prop) } - Expr::New(NewExpr { - callee: box callee, - args, - .. - }) => { + Expr::New(NewExpr { callee, args, .. }) => { let args = args.as_deref().unwrap_or(&[]); // We currently do not handle spreads. if args.iter().any(|arg| arg.spread.is_some()) { @@ -402,7 +398,7 @@ impl EvalContext { } Expr::Call(CallExpr { - callee: Callee::Expr(box callee), + callee: Callee::Expr(callee), args, .. }) => { @@ -505,13 +501,13 @@ impl EvalContext { PropOrSpread::Spread(SpreadElement { expr, .. }) => { ObjectPart::Spread(self.eval(arena, expr)) } - PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp { key, box value })) => { + PropOrSpread::Prop(Prop::KeyValue(KeyValueProp { key, value })) => { ObjectPart::KeyValue( self.eval_prop_name(arena, key), self.eval(arena, value), ) } - PropOrSpread::Prop(box Prop::Shorthand(ident)) => ObjectPart::KeyValue( + PropOrSpread::Prop(Prop::Shorthand(ident)) => ObjectPart::KeyValue( ident.sym.clone().into(), self.eval(arena, &Expr::Ident(ident.clone())), ), diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/visitor.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/visitor.rs index 64713f8665ab..4ae8abe01063 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/visitor.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/graph/visitor.rs @@ -1259,7 +1259,7 @@ impl<'a> Analyzer<'a, '_> { Some(path) } Expr::Arrow(ArrowExpr { - body: box BlockStmtOrExpr::BlockStmt(_), + body: BlockStmtOrExpr::BlockStmt(_), .. }) => { let mut path = as_parent_path(&ast_path); @@ -1272,7 +1272,7 @@ impl<'a> Analyzer<'a, '_> { Some(path) } Expr::Arrow(ArrowExpr { - body: box BlockStmtOrExpr::Expr(_), + body: BlockStmtOrExpr::Expr(_), .. }) => { let mut path = as_parent_path(&ast_path); @@ -1343,7 +1343,7 @@ impl<'a> Analyzer<'a, '_> { export_usage, }); } - Callee::Expr(box expr) => { + Callee::Expr(expr) => { if let Expr::Member(MemberExpr { obj, prop, .. }) = unparen(expr) { let obj_value = BumpBox::new_in(self.eval_context.eval(self.arena, obj), self.arena); @@ -3116,7 +3116,7 @@ impl<'a> Analyzer<'a, '_> { self.add_value( key.to_id(), - if let Some(box value) = value { + if let Some(value) = value { let value = self.eval_context.eval(self.arena, value); JsValue::alternatives(BumpVec::from_iter_in( self.arena, diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs index 970bbd948a7a..c76bbbb10e85 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/imports.rs @@ -1400,7 +1400,7 @@ impl Visit for Analyzer<'_> { MemberProp::Ident(..) | MemberProp::PrivateName(..) | MemberProp::Computed(ComputedPropName { - expr: box Expr::Lit(Lit::Str(_)), + expr: Expr::Lit(Lit::Str(_)), .. }) ) && let Expr::Ident(ident) = &*node.obj diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/well_known/require_context.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/well_known/require_context.rs index 583185ac93af..ef0facdda3ee 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/well_known/require_context.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/well_known/require_context.rs @@ -47,7 +47,7 @@ pub fn parse_require_context(args: &[JsValue<'_>]) -> Result, +) -> Result> { + let options = module.options().await?; + let parsed = module.failsafe_parse().await?; + let ParseResult::Ok { + program, + globals, + eval_context, + comments, + .. + } = &*parsed + else { + return Ok(ModuleSideEffects::SideEffectful.cell()); + }; + + let directives = parse_module_turbopack_directives(program); + let side_effects = if directives.no_side_effects { + ModuleSideEffects::SideEffectFree + } else if directives.constants_module && options.cross_module_constants { + // If the module is marked as a constants module, it must be side effect free, otherwise + // constant folding would not be safe. + ModuleSideEffects::SideEffectFree + } else if options.infer_module_side_effects { + GLOBALS.set(globals, || { + compute_module_evaluation_side_effects(program, comments, eval_context.unresolved_mark) + }) + } else { + ModuleSideEffects::SideEffectful + }; + + Ok(side_effects.cell()) +} + impl EcmascriptModuleAsset { pub fn analyze(self: Vc) -> Vc { analyze_ecmascript_module(self, None) @@ -864,7 +904,7 @@ impl Module for EcmascriptModuleAsset { { SideEffectsDeclaration::SideEffectful => ModuleSideEffects::SideEffectful, SideEffectsDeclaration::SideEffectFree => ModuleSideEffects::SideEffectFree, - SideEffectsDeclaration::None => self.analyze().await?.side_effects, + SideEffectsDeclaration::None => *compute_ecmascript_module_side_effects(self).await?, }) .cell()) } diff --git a/turbopack/crates/turbopack-ecmascript/src/module_fragments/graph.rs b/turbopack/crates/turbopack-ecmascript/src/module_fragments/graph.rs index 33e7cb2a9130..cedcd4c8c69f 100644 --- a/turbopack/crates/turbopack-ecmascript/src/module_fragments/graph.rs +++ b/turbopack/crates/turbopack-ecmascript/src/module_fragments/graph.rs @@ -642,7 +642,7 @@ impl DepGraph { // Skip directives, as we copy them to each modules. if let ModuleItem::Stmt(Stmt::Expr(ExprStmt { - expr: box Expr::Lit(Lit::Str(s)), + expr: Expr::Lit(Lit::Str(s)), .. })) = &data[g].content && s.value.starts_with("use ") @@ -1367,7 +1367,7 @@ impl DepGraph { } ModuleItem::Stmt(Stmt::Expr(ExprStmt { - expr: box Expr::Assign(assign), + expr: Expr::Assign(assign), .. })) => { let mut used_ids = ids_used_by_ignoring_nested( @@ -1680,17 +1680,17 @@ pub(crate) fn create_turbopack_part_id_assert(dep: PartId) -> ObjectLit { pub(crate) fn find_turbopack_part_id_in_asserts(asserts: &ObjectLit) -> Option { asserts.props.iter().find_map(|prop| match prop { - PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp { + PropOrSpread::Prop(Prop::KeyValue(KeyValueProp { key: PropName::Ident(key), - value: box Expr::Lit(Lit::Num(chunk_id)), + value: Expr::Lit(Lit::Num(chunk_id)), })) if &*key.sym == ASSERT_CHUNK_KEY => Some(PartId::Internal( chunk_id.value.abs() as u32, chunk_id.value.is_sign_positive(), )), - PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp { + PropOrSpread::Prop(Prop::KeyValue(KeyValueProp { key: PropName::Ident(key), - value: box Expr::Lit(Lit::Str(s)), + value: Expr::Lit(Lit::Str(s)), })) if &*key.sym == ASSERT_CHUNK_KEY => match s.value.as_str()? { "module evaluation" => Some(PartId::ModuleEvaluation), "exports" => Some(PartId::Exports), diff --git a/turbopack/crates/turbopack-ecmascript/src/module_fragments/mod.rs b/turbopack/crates/turbopack-ecmascript/src/module_fragments/mod.rs index c5d4d677fc67..7f087bfe59a7 100644 --- a/turbopack/crates/turbopack-ecmascript/src/module_fragments/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/module_fragments/mod.rs @@ -524,7 +524,7 @@ pub(super) async fn split_module(asset: Vc) -> Result { + ModuleDecl::ExportDefaultExpr(ExportDefaultExpr { expr, .. }) => { let decl = Decl::Var(Box::new(VarDecl { span: DUMMY_SP, ctxt: Default::default(), @@ -74,7 +74,7 @@ impl EsmModuleItem { Default::default(), ) .into(), - init: Some(Box::new(expr)), + init: Some(expr), definite: false, }], })); diff --git a/turbopack/crates/turbopack-ecmascript/src/references/esm/url.rs b/turbopack/crates/turbopack-ecmascript/src/references/esm/url.rs index 351f872f557b..ebcfb696b016 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/esm/url.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/esm/url.rs @@ -271,21 +271,17 @@ impl UrlAssetReferenceCodeGen { args: Some(args), .. }) = new_expr { - if let Some(ExprOrSpread { - box expr, - spread: None, - }) = args.get_mut(0) + if let Some(ExprOrSpread { expr, spread: None }) = + args.get_mut(0) { - *expr = url_segment_resolver.clone(); + **expr = url_segment_resolver.clone(); } - if let Some(ExprOrSpread { - box expr, - spread: None, - }) = args.get_mut(1) + if let Some(ExprOrSpread { expr, spread: None }) = + args.get_mut(1) { if let Some(rewrite) = &rewrite_url_base { - *expr = rewrite.clone(); + **expr = rewrite.clone(); } else { // If rewrite for the base doesn't exists, means // __turbopack_resolve_module_id_path__ @@ -308,21 +304,17 @@ impl UrlAssetReferenceCodeGen { args: Some(args), .. }) = new_expr { - if let Some(ExprOrSpread { - box expr, - spread: None, - }) = args.get_mut(0) + if let Some(ExprOrSpread { expr, spread: None }) = + args.get_mut(0) { *expr = request.as_str().into() } if let Some(rewrite) = &rewrite_url_base - && let Some(ExprOrSpread { - box expr, - spread: None, - }) = args.get_mut(1) + && let Some(ExprOrSpread { expr, spread: None }) = + args.get_mut(1) { - *expr = rewrite.clone(); + **expr = rewrite.clone(); } } } diff --git a/turbopack/crates/turbopack-ecmascript/src/references/mod.rs b/turbopack/crates/turbopack-ecmascript/src/references/mod.rs index e414ef55724a..0819ea2fe366 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/mod.rs @@ -80,7 +80,7 @@ use turbopack_core::{ }, environment::Rendering, issue::{IssueExt, IssueSeverity, IssueSource, StyledString, analyze::AnalyzeIssue}, - module::{Module, ModuleSideEffects}, + module::Module, reference::{ModuleReference, ModuleReferences}, reference_type::{CommonJsReferenceSubType, InnerAssets}, resolve::{ @@ -109,13 +109,12 @@ use crate::{ graph::{ConditionalKind, Effect, EffectArg, VarGraph, create_graph}, imports::{ImportAnnotations, ImportAttributes, ImportMap}, linker::link, - parse_require_context, side_effects, + parse_require_context, top_level_await::has_top_level_await, well_known::replace_well_known, }, chunk::CjsStaticExports, code_gen::{CodeGen, CodeGens, IntoCodeGenReference}, - directive::parse_module_turbopack_directives, errors, module_fragments::{part_of_module, split_module}, parse::ParseResult, @@ -170,7 +169,6 @@ pub struct AnalyzeEcmascriptModuleResult { pub code_generation: ResolvedVc, pub async_module: ResolvedVc, - pub side_effects: ModuleSideEffects, /// `true` when the analysis was successful. pub successful: bool, pub source_map: Option>>, @@ -235,7 +233,6 @@ struct AnalyzeEcmascriptModuleResultBuilder { async_module: ResolvedVc, successful: bool, source_map: Option>>, - side_effects: ModuleSideEffects, cjs_static_exports: Option, env_var_info_runtime: FxIndexSet, @@ -258,7 +255,6 @@ impl AnalyzeEcmascriptModuleResultBuilder { async_module: ResolvedVc::cell(None), successful: false, source_map: None, - side_effects: ModuleSideEffects::SideEffectful, cjs_static_exports: None, env_var_info_runtime: Default::default(), #[cfg(debug_assertions)] @@ -332,11 +328,6 @@ impl AnalyzeEcmascriptModuleResultBuilder { self.async_module = ResolvedVc::cell(Some(async_module)); } - /// Set whether this module is side-effect free according to a user-provided directive. - pub fn set_side_effects_mode(&mut self, value: ModuleSideEffects) { - self.side_effects = value; - } - /// Sets whether the analysis was successful. pub fn set_successful(&mut self, successful: bool) { self.successful = successful; @@ -449,7 +440,6 @@ impl AnalyzeEcmascriptModuleResultBuilder { ), code_generation: ResolvedVc::cell(code_generation), async_module: self.async_module, - side_effects: self.side_effects, successful: self.successful, source_map: self.source_map, cjs_static_exports: self.cjs_static_exports, @@ -710,28 +700,6 @@ async fn analyze_ecmascript_module_internal( analysis.add_esm_evaluation_reference(*i); } - let directives = parse_module_turbopack_directives(program); - analysis.set_side_effects_mode(if directives.no_side_effects { - ModuleSideEffects::SideEffectFree - } else if directives.constants_module && options.cross_module_constants { - // If the module is marked as a constants module, it must be side effect free, otherwise - // the constant folding would not be safe. This makes a difference when doing `import * - // as foo from 'constants-module'` - ModuleSideEffects::SideEffectFree - } else if options.infer_module_side_effects { - // Analyze the AST to infer side effects - GLOBALS.set(globals, || { - side_effects::compute_module_evaluation_side_effects( - program, - comments, - eval_context.unresolved_mark, - ) - }) - } else { - // If inference is disabled, assume side effects - ModuleSideEffects::SideEffectful - }); - let is_esm = eval_context.is_esm(specified_type); let compile_time_info = compile_time_info_for_module_options( diff --git a/turbopack/crates/turbopack-ecmascript/src/utils.rs b/turbopack/crates/turbopack-ecmascript/src/utils.rs index 98e42635b4e7..d3850165e665 100644 --- a/turbopack/crates/turbopack-ecmascript/src/utils.rs +++ b/turbopack/crates/turbopack-ecmascript/src/utils.rs @@ -35,7 +35,7 @@ pub(crate) fn extract_name_from_member_prop(prop: &MemberProp) -> Option Some(SmallVec::from_buf([ident.sym.as_str().into()])), MemberProp::Computed(ComputedPropName { - expr: box Expr::Lit(Lit::Str(s)), + expr: Expr::Lit(Lit::Str(s)), .. }) => s.value.as_str().map(|v| SmallVec::from_buf([v.into()])), _ => None, @@ -83,7 +83,7 @@ pub fn js_value_to_pattern(value: &JsValue<'_>) -> Pattern { ConstantValue::Null => rcstr!("null"), ConstantValue::Num(ConstantNumber(n)) => n.to_string().into(), ConstantValue::BigInt(n) => n.to_string().into(), - ConstantValue::Regex(box (exp, flags)) => format!("/{exp}/{flags}").into(), + ConstantValue::Regex((exp, flags)) => format!("/{exp}/{flags}").into(), ConstantValue::Undefined => rcstr!("undefined"), }), JsValue::Url(v, JsValueUrlKind::Relative) => Pattern::Constant(v.as_rcstr()), diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/A.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/A.ts new file mode 100644 index 000000000000..7ba2ffc4e861 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/A.ts @@ -0,0 +1,12 @@ +import { B } from './B' + +// This re-export is what forces the facade/locals split for this module +// (`EcmascriptExports::split_locals_and_reexports` returns true as soon as a +// module has any `ImportedBinding`/star re-export). No other option is needed. +export { helper } from './helper' + +export function A(n: number) { + if (n > 0) { + B(n - 1) + } +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/B.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/B.ts new file mode 100644 index 000000000000..ccba36c34d3f --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/B.ts @@ -0,0 +1,11 @@ +import { C } from './C' +import { asyncFn } from './asyncFn' + +export { helper } from './helper' + +export function B(n: number) { + if (n > 0) { + C(n - 1) + asyncFn() + } +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/C.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/C.ts new file mode 100644 index 000000000000..b56f8ab35ba6 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/C.ts @@ -0,0 +1,9 @@ +import { A } from './A' + +export { helper } from './helper' + +export function C(n: number) { + if (n > 0) { + A(n - 1) + } +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/asyncFn.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/asyncFn.js new file mode 100644 index 000000000000..b808ca509496 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/asyncFn.js @@ -0,0 +1,8 @@ +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} +// Top-level await makes this an async module, which is what drags the whole +// import cycle into async-module handling. +await sleep(0) + +export function asyncFn() {} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/helper.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/helper.ts new file mode 100644 index 000000000000..c52e6a68ad02 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/helper.ts @@ -0,0 +1,3 @@ +export function helper() { + return 'helper' +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/index.js new file mode 100644 index 000000000000..1383a9712b3c --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-async-deadlock/input/index.js @@ -0,0 +1,36 @@ +import { A } from './A' + +/* + * Regression test: turbopack used to hang (deadlock) while building the module + * graph for an import cycle whose modules are split into facade + locals + * modules. + * + * Topology (same shape as ../../async-modules/cycle-2, plus re-exports): + * + * index -> A + * ^\ + * | v + * C<-B -> asyncFn (top-level await) + * + * A, B and C each carry `export { helper } from './helper'`. That re-export is + * enough to make `EcmascriptExports::split_locals_and_reexports` return true, + * so each of them is split into an `EcmascriptModuleFacadeModule` plus an + * `EcmascriptModuleLocalsModule`. + * + * Resolving `import { A } from './A'` goes through `apply_reexport_tree_shaking` + * (module resolution, `turbopack/src/lib.rs`), which calls + * `follow_reexports(A_facade, "A")`. That walks facade -> locals, and the locals + * step used to ask the locals module for its side effects, which are derived + * from the original module's `analyze()` — already in flight further up the same + * import cycle. The result was a turbo-tasks await cycle: the process sat at + * ~0.5% CPU with completely flat RSS and never finished, so `next build` would + * hang with no output and no error. + * + * The async module is not required to trigger this; see + * `../reexport-cycle-deadlock` for the same cycle without a top-level `await`. + */ + +it('should not deadlock building a re-exporting import cycle with an async module', () => { + A(10) + expect(true).toBe(true) +}) diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/A.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/A.ts new file mode 100644 index 000000000000..7ba2ffc4e861 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/A.ts @@ -0,0 +1,12 @@ +import { B } from './B' + +// This re-export is what forces the facade/locals split for this module +// (`EcmascriptExports::split_locals_and_reexports` returns true as soon as a +// module has any `ImportedBinding`/star re-export). No other option is needed. +export { helper } from './helper' + +export function A(n: number) { + if (n > 0) { + B(n - 1) + } +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/B.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/B.ts new file mode 100644 index 000000000000..02a659cccd00 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/B.ts @@ -0,0 +1,11 @@ +import { C } from './C' +import { syncFn } from './syncFn' + +export { helper } from './helper' + +export function B(n: number) { + if (n > 0) { + C(n - 1) + syncFn() + } +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/C.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/C.ts new file mode 100644 index 000000000000..b56f8ab35ba6 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/C.ts @@ -0,0 +1,9 @@ +import { A } from './A' + +export { helper } from './helper' + +export function C(n: number) { + if (n > 0) { + A(n - 1) + } +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/helper.ts b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/helper.ts new file mode 100644 index 000000000000..c52e6a68ad02 --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/helper.ts @@ -0,0 +1,3 @@ +export function helper() { + return 'helper' +} diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/index.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/index.js new file mode 100644 index 000000000000..5de4f384222a --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/index.js @@ -0,0 +1,28 @@ +import { A } from './A' + +/* + * Regression test: turbopack used to hang (deadlock) while building the module + * graph for an import cycle whose modules are split into facade + locals + * modules. + * + * Topology: + * + * index -> A + * ^\ + * | v + * C<-B -> syncFn + * + * A, B and C each carry `export { helper } from './helper'`. That re-export is + * enough to make `EcmascriptExports::split_locals_and_reexports` return true, + * so each of them is split into an `EcmascriptModuleFacadeModule` plus an + * `EcmascriptModuleLocalsModule`. + * + * This is the same bug as `../reexport-cycle-async-deadlock`, but without any + * async module: a top-level `await` anywhere in the cycle is not required to + * trigger it. The re-export cycle alone is enough. + */ + +it('should not deadlock building a re-exporting import cycle', () => { + A(10) + expect(true).toBe(true) +}) diff --git a/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/syncFn.js b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/syncFn.js new file mode 100644 index 000000000000..93ce2b97cf6c --- /dev/null +++ b/turbopack/crates/turbopack-tests/tests/execution/turbopack/side-effect-optimization/reexport-cycle-deadlock/input/syncFn.js @@ -0,0 +1 @@ +export function syncFn() {} diff --git a/turbopack/crates/turbopack-trace-server/src/lib.rs b/turbopack/crates/turbopack-trace-server/src/lib.rs index a3874769b37a..55b88ebc3819 100644 --- a/turbopack/crates/turbopack-trace-server/src/lib.rs +++ b/turbopack/crates/turbopack-trace-server/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(box_patterns)] +#![feature(deref_patterns)] #![feature(bufreader_peek)] use std::{ diff --git a/turbopack/crates/turbopack-trace-server/src/main.rs b/turbopack/crates/turbopack-trace-server/src/main.rs index ecb62d28bb48..0cd702563c07 100644 --- a/turbopack/crates/turbopack-trace-server/src/main.rs +++ b/turbopack/crates/turbopack-trace-server/src/main.rs @@ -1,4 +1,4 @@ -#![feature(box_patterns)] +#![feature(deref_patterns)] #![feature(bufreader_peek)] #[global_allocator] diff --git a/turbopack/crates/turbopack-trace-server/src/self_time_tree.rs b/turbopack/crates/turbopack-trace-server/src/self_time_tree.rs index 6e9d4c36698e..e600951ec2c9 100644 --- a/turbopack/crates/turbopack-trace-server/src/self_time_tree.rs +++ b/turbopack/crates/turbopack-trace-server/src/self_time_tree.rs @@ -135,7 +135,7 @@ impl SelfTimeTree { } fn rebalance(&mut self) { - if let Some(box SelfTimeChildren { + if let Some(SelfTimeChildren { left, split_point, right, @@ -159,7 +159,7 @@ impl SelfTimeTree { // right' = (left.right, right) with self.split_point // split_point' = left.split_point // direct entries in self and left are put in self and are redistributed - if let Some(box SelfTimeChildren { + if let Some(SelfTimeChildren { left: left_left, split_point: left_split_point, right: left_right, @@ -189,7 +189,7 @@ impl SelfTimeTree { // right' = right.right // split_point' = right.split_point // direct entries in self and right are put in self and are redistributed - if let Some(box SelfTimeChildren { + if let Some(SelfTimeChildren { left: right_left, split_point: right_split_point, right: right_right,