Luke SandbergandGitHub 0f38c522bc Turbopack: simplify asset ident constructors (#93213) ...

### What?

Removes the per-method turbo-task constructors on `AssetIdent` (`from_path`, `with_query`, `with_fragment`, `with_modifier`, `with_part`, `with_path`, `with_layer`, `with_content_type`, `with_asset`, `rename_as`, and `path`). Each of those was its own cached task that returned a small projection or a one-field-changed copy. They are now plain Rust builder methods on the owned value, with a single `into_vc()` at the end of the chain that goes through the existing cached `new_inner` constructor.

Call sites that previously chained `Vc` methods now look like:

```rust
module
    .ident()
    .owned()
    .await?
    .with_modifier(rcstr!("async loader"))
    .into_vc()
```

### Why?

These constructors were tiny "projection" turbo-tasks that paid the cost of a task lookup, cell allocation, and dependency tracking but whose cache layer didn't meaningfully prevent recomputation. The trade-off is invalidation semantics:

- **Before:** a caller doing `module.ident().path()` depended on the cached `path()` projection. If the source `AssetIdent` changed but its `.path` field was unchanged (e.g. a new modifier was added), `path()` re-ran, returned the same `FileSystemPath` cell, and the caller did not re-run.
- **After:** the same caller does `module.ident().await?.path` and depends directly on the `AssetIdent` cell. Any change to the ident (modifier, query, layer, …) invalidates the caller, even if the path is unchanged.

In practice this is rarely a real loss: when an ident changes, the `Module` typically changes too, and the dependent task was going to re-run anyway. `new_inner` already deduplicates structurally-equal idents, so the wrappers were paying overhead per call without buying meaningful invalidation isolation.

Measured on a `vercel-site` build via `NEXT_TURBOPACK_TASK_STATISTICS` and `turbopack/scripts/analyze_cache_effectiveness.py`:

| Task                              | canary (hits / misses) | this branch |
| --------------------------------- | ---------------------- | ----------- |
| `AssetIdent::path`                | 778,273 / 98,018       | removed     |
| `AssetIdent::with_modifier`       | 27,895 / 22,801        | removed     |
| `AssetIdent::from_path`           | 2,954 / 29,650         | removed     |
| `AssetIdent::with_part`           | 2 / 5,440              | removed     |
| `AssetIdent::with_layer`          | 7 / 4,356              | removed     |
| `AssetIdent::rename_as`           | 4,969 / 2,269          | removed     |
| `AssetIdent::with_query`          | 0 / 521                | removed     |
| `AssetIdent::with_content_type`   | 0 / 79                 | removed     |
| `AssetIdent::new_inner`           | 628 / 129,777          | 29,213 / 120,650 |

Aggregate over the whole build:

- Total cached tasks: 1,300 → 1,292
- Total task invocations: 39,361,186 → 38,208,036 (~1.15M fewer lookups)
- Total cache misses: 6,812,198 → 6,639,937 (~172k fewer)
- Overall hit rate: 82.7% → 82.6% (essentially unchanged)

`new_inner` absorbs the construction work that used to be split across the wrappers. Four upstream tasks gained +519 cache hits each (`EsmAssetReference::resolve_reference`, `ReferencedAsset::from_resolve_result`, `NextServerUtilityModule::ident`, `NodeJsChunkingContext::chunk_item_id_strategy`); no task gained any new misses.

### How?

- `AssetIdent::from_path` and the `with_*` methods are now plain `&mut self`/`self`-by-value builder methods on the struct itself, not `#[turbo_tasks::function]`s.
- A new `AssetIdent::into_vc(self)` finalizes the builder by going through the still-cached `new_inner`.
- `AssetIdent::path()` is removed; callers use `.path` on an owned `AssetIdent`.
- All call sites across `turbopack-*` and `next-*` crates are updated. Most go from `ident.with_modifier(m)` (returning `Vc`) to `ident.owned().await?.with_modifier(m).into_vc()`.
- A follow-up commit removes a few `.clone()`s introduced in the conversion that aren't needed once lifetimes are bound to a local.

### Follow-ups (out of scope)

While migrating call sites, two pre-existing entry builders surfaced as candidates for cleanup. Not addressed here, but worth noting:

- `get_app_page_entry` (`crates/next-core/src/next_app/app_page_entry.rs`) replaces the *content* of the source returned by `load_next_js_template` (prefixing imports onto `result.build()`) but reuses the template's `ident` with a `?page=...` query suffix as a disambiguator. The new `VirtualSource` ends up with content from one place and an ident chain pointing at another. A cleaner shape would be to mint a fresh ident from the page path, since the caller already knows what it's building.
- `create_page_ssr_entry_module` (`crates/next-pages/page_entry.rs`) has the same shape on the instrumentation-conflict branch: it appends `export const register = hoist(...)` to the template content and constructs a `VirtualSource` with the original `source.ident()` unchanged. Lower-frequency than the app-page case (fires at most once per build), but the ident still misrepresents the constructed content.

<!-- NEXT_JS_LLM_PR -->

2026-05-06 10:38:07 -07:00

..

src

Turbopack: simplify asset ident constructors (#93213)

2026-05-06 10:38:07 -07:00

Cargo.toml

…