[pull] canary from vercel:canary - #1398
Merged
Merged
Conversation
### What? Introduce `EsmAssetReferenceOptions`, an explicit options object for ESM asset references. ### Why? `EsmAssetReference::new` and `new_pure` accepted a long positional list of behavioral settings. Call sites were difficult to scan and easy to misconfigure as the reference grows new behavior. This is intentionally a behavior-neutral, independently reviewable base for the two existing tree-shaking PRs above it: 1. This PR centralizes constructor configuration. 2. #96282 uses the options object to configure re-export usage forwarding. 3. #96396 tracks static member reads through namespace-valued re-exports. ### How? Keep the three reference-identity inputs (`module`, `origin`, and `request`) positional, and move behavioral settings—source location, annotations, target part, import usage, externals, module-fragment mode, and resolve override—into a named options struct. The two constructor call sites now declare their settings by field name. The refactor removes the `too_many_arguments` allowances without changing resolution or binding-usage behavior. ### Testing - `cargo check -p turbopack-ecmascript` - Analyzer graph tests: 60/60 - The stacked focused execution/snapshot tests, 2/2 usage-lattice units, `build-all`, and HMR regression are verified on the upper PRs. <!-- NEXT_JS_LLM --> <!-- fleet 993ef5d6-cc80-4082-8e14-ff9ac871ce33 --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
## What? This PR improves tree-shaking across whole-namespace re-exports. It is the middle PR in the stack: #98680 provides the independently reviewable `EsmAssetReferenceOptions` constructor refactor requested during review, and #96396 builds on this PR to narrow static member reads on namespace-valued named exports. A representative case is a CommonJS re-export followed by an ESM star barrel: ```js // index.js const { m } = require('./cjs') // cjs.js module.exports = require('./barrel') // CJS re-export // barrel.js export * from './base' // ESM star barrel // base.js export const m = 1 export const n = 2 // nobody ever uses this ``` Only `m` is observed by the entrypoint, so that usage can now flow through both re-export layers to `base.js` and allow `n` to be removed. ## Why? Turbopack now has a generic `ExportUsage::Passthrough` mechanism, introduced on `canary` for client-component and Next.js module proxies. It carries the referencing module's accumulated used-export set into a transparent target. However, CommonJS whole-module forwarding and ordinary ESM star barrels do not opt into that mechanism. Applying this PR's tests to plain `canary` showed that all runtime values are already correct, but unused exports remain retained in four important shapes: - an ESM named consumer through `module.exports = require(ESM)`; - a deep CommonJS forwarding chain; - CommonJS forwarding into an ESM `export *` barrel; - a CommonJS diamond with disjoint used exports. This PR therefore remains a tree-shaking precision change rather than introducing a second forwarding model. There is also an independent name-observability concern. A CommonJS forwarder exposes a raw namespace object's properties under their original names, while named consumers of an ESM `export *` can be rewritten if an export is mangled. Namespace provenance must survive multiple passthrough hops without unnecessarily disabling mangling for ordinary ESM named forwarding. ## How? ### Configure forwarding at construction time #98680 replaces `EsmAssetReference`'s positional behavior arguments with `EsmAssetReferenceOptions`. This PR extends that options object with an optional passthrough mode. ESM star references receive `Some(false)` directly when constructed; free-variable references receive `None`; and the existing annotation-driven client-proxy path still contributes `Some(true)`. The two sources merge conservatively: an annotation that exposes the namespace's original names cannot be weakened by a syntax-driven `Some(false)` on the same reference. The former mutating `mark_namespace_reexport()` API is removed. ### Reuse the generic passthrough fixed point The implementation uses canary's existing model: ```rust ExportUsage::Passthrough { namespace_object_may_escape: bool, } ``` The boolean describes whether the edge itself exposes the target namespace's original property names: - `module.exports = require('…')` records `Passthrough { namespace_object_may_escape: true }` directly in the existing CommonJS import-usage map; - `export * from '…'` records `Passthrough { namespace_object_may_escape: false }`, because statically known ESM consumers can be rewritten when target exports are mangled. No parallel `TargetExportUsage` type, graph field, call parameter, or second fixed-point implementation is added. Detection is limited to a simple assignment using the unresolved/global `module` and `require` bindings. Shadowed bindings, member requires, and compound assignments retain their existing behavior. ### Propagate keys and namespace provenance independently A passthrough edge merges the parent module's resolved `Evaluation`, `Exports`, or `All` state into its target. The merge remains monotonic and idempotent, so chains, cycles, and diamonds converge. Namespace provenance is carried independently of the used key set: - an edge with `namespace_object_may_escape: true` makes the target's original names observable; - every passthrough also carries namespace provenance that already reached its parent; - a newly propagated provenance bit retriggers traversal even when the used key set did not change. This preserves original names through multi-hop namespace reads while keeping unrelated ordinary named exports mangleable. Passthrough references are also exposed correctly to scope-hoisted merged modules. ## Testing Enabled execution coverage includes: - deep CommonJS chains, CommonJS → ESM star barrels, cycles, diamonds, and overwrite behavior; - ESM named/default/namespace import interop through `module.exports = require(ESM)`; - opaque whole-namespace and evaluation-only consumption, including single evaluation; - native `export *`, default/local-shadow filtering, and the optimization-disabled mode; - namespace provenance across forwarding edges and a multi-hop diamond; - both mangling safety and non-pessimization for ordinary named forwarding; - shadowed `module`/`require`, member-require, and non-simple assignment boundaries; - runtime counterparts for every nested precision case that remains skipped. Known optimization-only limitations remain under `__skipped__/deep-reexports`: nested `export * as` through CJS, namespace-property unwrapping in a CJS assignment, depth-two nested namespaces, and unwrapping after a forwarding hop. Their runtime semantics are covered by the enabled `reexport-forwarding-nested-runtime` fixture. Verification on the rebased stack: - plain-canary reassessment: 7/9 fixtures, with only four expected precision failures; - integrated bottom layer: 9/9 forwarding fixtures; - passthrough unit tests: 2/2, plus the six-case construction-option merge lattice; - combined stack: 14/14 focused execution/snapshot tests and 60/60 analyzer graph tests; - affected Cargo checks, `next-core` check, `build-all`, and the forwarding/namespace HMR test pass. <!-- NEXT_JS_LLM --> <!-- fleet 993ef5d6-cc80-4082-8e14-ff9ac871ce33 --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
### What? Migrate the five local isolated test installs that explicitly used npm to pnpm. The Nx fixture now has pnpm workspace metadata, while filesystem-layout-sensitive fixtures use pnpm's hoisted linker with copied package files. ### Why? The npm-based Nx install bypassed the repository's centralized supply-chain protections and could select a package immediately after publication, including temporarily incomplete multi-package releases. Using pnpm makes isolated installs inherit the repository's `minimumReleaseAge`, exclusions, and exotic-subdependency policy. The other npm installs depended on npm-style real package directories. On Node versions affected by nodejs/node#65113, hoisted/copy mode preserves that layout without leaving these fixtures outside the shared pnpm security configuration; fixed Node releases use normal pnpm linking. ### How? - Use normal pnpm workspace resolution for the Nx fixture. - Use `node-linker=hoisted` and `package-import-method=copy` for filesystem tests only on affected Node releases; Node 24.21+ and 26.8+ use normal linking. Node 20 CI keeps the workaround because no fixed Node 20 release exists. - Validate local `@next/env` tarballs through the lockfile when hoisted installs do not expose pnpm's virtual-store path marker. - Keep the deployment-environment npm install unchanged. ### Verification - `pnpm build-all` - `pnpm types` - A 9-version throwaway assertion verified the affected/fixed Node release matrix - `pnpm test-dev-turbo test/e2e/app-dir/nx-handling/nx-handling.test.ts test/e2e/handle-non-hoisted-swc-helpers/index.test.ts test/e2e/filesystem-cache/filesystem-cache.test.ts test/e2e/filesystem-cache/warm-restart-task-stats.test.ts test/e2e/filesystem-cache/evict-after-snapshot.test.ts` — all 25 tests passed after installing the sandbox's missing Playwright browser - Production Turbopack: Nx, non-hoisted SWC helper, build-cache-default, and warm restart passed (9/9) - `filesystem-cache.test.ts` production baseline: 15/17 passed; the same two cache-growth bounds fail under both the unchanged npm fixture and the pnpm fixture at nearly identical percentages, so they are pre-existing sandbox-specific failures - Generated-layout inspection: no package symlinks outside expected `.bin` command shims; package files are copied; `node_modules/.pnpm` is metadata-only <!-- NEXT_JS_LLM --> <!-- fleet 81cd457d-6956-4cf9-b6f6-9ebf9d95f285 --> --------- Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
## Summary Replace top-level consistency suppression in read-ref and trait-ref cell tests with explicit operation roots. The operations separately cover counter creation, counter reads, and trait upcasts while preserving the tests’ warm-cache and snapshot behavior. ## Verification - `cargo test -p turbo-tasks-backend --test read_ref_cell --test trait_ref_cell` - `cargo fmt --all -- --check` <!-- NEXT_JS_LLM --> <!-- fleet 475180cd-06a7-4669-89b5-a2029a3a18c9 --> --------- Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
### What?
Fixes constant replacement in object shorthand properties and adds
focused Turbopack execution coverage for the existing analyzer-aware
cross-module constants path.
### Why?
This is a pre-existing bug in `ConstantValueCodeGen`, which is shared by
the current analyzer-aware/opt-in constants implementation and the newer
codegen-only export-inlining work. When an imported constant is used as
`{ VALUE }`, the AST path ends at `Prop::Shorthand`; the old
expression-only visitor could replace the enclosing object literal,
collapsing `{ FIRST, SECOND }` to a single primitive.
This fix is intentionally independent of the export-inlining feature
stack so existing `turbopackCrossModuleConstants` users receive and
review the correction separately.
### How?
When the code-generation path ends at a shorthand property, it now
mirrors `EsmBinding`: the shorthand is expanded to a key/value property,
preserving the original key and generating the compile-time constant
only for the value. Other expression paths are unchanged, and generated
values retain the standard compile-time marker.
The execution fixture uses only analyzer-aware constants. Without the
fix it returns `"second"` instead of `{ FIRST: "first", SECOND: "second"
}`; with the fix it verifies the runtime object, both generated keys,
and both marked inlined values.
### Verification
- `NODE_PATH=/vercel/sandbox/test-deps/node_modules cargo test -p
turbopack-tests --test execution cross_module_constants_shorthand --
--nocapture`
- `cargo test -p turbopack-tests --test snapshot cross_module --
--nocapture`
- `cargo check -p turbopack-ecmascript -p turbopack-tests --tests`
- `cargo fmt --all -- --check`
<!-- NEXT_JS_LLM -->
<!-- fleet 29433e1c-e26e-435a-b516-45108f629d79 -->
Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com>
Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
### What? Use pnpm's default package import method in the two hoisted isolated-test configurations added by #98425. ### Why? `package-import-method` controls how regular package files are materialized from pnpm's store: by reflink, hardlink, or copy. Those choices do not affect `realpath`, so forcing copies is unrelated to the Node.js symlink-resolution workaround and unnecessarily disables pnpm's more efficient defaults. ### How? Keep `node-linker=hoisted`, which is the setting responsible for producing npm-style real package directories, while removing the independent copy policy. The Node-version gate, local-tarball validation, release-age policy, and fixture behavior remain unchanged. ### Verification - Hoisted-only scratch install produced real package directories, no package symlinks outside `.bin`, and a metadata-only `.pnpm` - Production Turbopack: non-hoisted SWC helper and warm-restart task stats passed (2/2) - `pnpm build` - `pnpm types` - Prettier, ESLint, and `git diff --check` Follow-up to #98425 and #98425 (comment). <!-- NEXT_JS_LLM --> <!-- fleet 81cd457d-6956-4cf9-b6f6-9ebf9d95f285 --> Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
### What? Makes Turbopack import tracing recognize the module graph's explicit entries as roots, including when an entry participates in an import cycle. Adds focused graph-level coverage and a Next.js production fixture that exercises issue formatting through a cyclic graph. If a malformed graph still has no path to any explicit entry, tracing now returns a minimal trace and emits a bug-severity issue instead of panicking. ### Why? The import tracer inferred roots from nodes with no incoming edges. A valid cycle that points back to an entry gives every node an incoming edge, so the tracer could not find a root and panicked while formatting another diagnostic. The graph already records its entries explicitly, making them the authoritative and cycle-safe definition of a root. The same topology-based assumption affected entry membership checks, so those now use the explicit entry list as well. ### How? - Resolve import-trace paths against node indices derived from `GraphEntries`. - Preserve a defensive fallback for malformed graphs and classify that invariant violation as an implementation bug. - Cover cyclic entry paths, malformed rootless graphs, absent paths, and the product-level issue-formatting path. Fixes #98205 <!-- NEXT_JS_LLM --> <!-- fleet 25b26044-3397-4642-a77f-c565b7577046 --> --------- Co-authored-by: vercel-fleet-prod[bot] <318278635+vercel-fleet-prod[bot]@users.noreply.github.com> Co-authored-by: Tobias Koppers <1365881+sokra@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )