Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] canary from vercel:canary #568

Open
wants to merge 6,368 commits into
base: canary
Choose a base branch
from
Open

Conversation

pull[bot]
Copy link

@pull pull bot commented Feb 8, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot requested a review from timneutkens as a code owner February 8, 2024 01:29
@pull pull bot added the ⤵️ pull label Feb 8, 2024
vercel-release-bot and others added 28 commits November 4, 2024 23:24
## Why?

The default cache config for fetch is `{ cache: 'no-store' }` with v15.

- Fixes #72213
…olvedVc (#72200)

A hand refactor of all the remaining struct fields from `Vc` to `ResolvedVc`, to unblock local tasks.

This is similar to #71948, as there's a focus on resolve options.

Note: "resolve" in this context means two different things. `turbopack-resolve` is about resolving module locations on disk. `ResolvedVc` is about resolving "cells" representing incremental computations.

Closes PACK-3369
This adds a new ResumeDataCache concept to Next.js. This cache is persisted during production, and used for pre-seeding during development. Every route will generate its own resume data cache that will be used when partial prerendering is enabled. This lets us fill the caches during the prerender, and re-use those cache entries every time the page is resumed. This allows a future where we can reuse stale static shells while also reusing the resume data cache.
This change is part of a larger set of refactors meant to eventually
remove the requesStore that is currently scoped around the entire
renderToHTML... function call. Ideally workUnitStore would be scoped to
specific render tasks and in this change I am disentangling one
dependency on this outer store by directly passing implicitTags into the
prerender implementation to remove one dependency on this outer store.

Stacked on #72145

This change should not alter any behavior
### Improving Documentation
- fix typo

`intented` -> `intended`

Co-authored-by: Delba de Oliveira <[email protected]>
### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

fix typo

myFeaturFlag.isEnabled() -> myFeatureFlag.isEnabled()

Co-authored-by: Delba de Oliveira <[email protected]>
### Improving Documentation

fix typo

Co-authored-by: Delba de Oliveira <[email protected]>
### Improving Documentation

- fix typo 

`compatability` -> `compatibility`

Co-authored-by: Delba de Oliveira <[email protected]>
## Why?

Our default CSS minifier had a bug → cssnano/cssnano#1655, which was also reported (below).

Since it is now fixed upstream, we need to update our cssnano in Next.js.

- Fixes #68476

## How?

- Update `cssnano-preset-default` to v6
- Add unit test to cover the case for being able to declare layer names

(the added unit test errors in an way that's unreadable on canary)
![CleanShot 2024-11-04 at 22 32 32@2x](https://github.com/user-attachments/assets/ef3a302a-2aec-4b63-9db2-40869daaf5ad)
## Description
At #71964, Legacy tag was introduced.
Then add that to `exportPathMap` docs instead of `deprecated` title
name.

In addition, remove [Static
Export](https://github.com/vercel/next.js/tree/canary/examples/with-static-export)
example because it's no more use `exportPathMap`.

### Improving Documentation

- [x] Run `pnpm prettier-fix` to fix formatting issues before opening
the PR.
- [x] Read the Docs Contribution Guide to ensure your contribution
follows the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

Co-authored-by: Delba de Oliveira <[email protected]>
## Summary
Add missing `## Useful Links` section header to adapt
[template](https://github.com/vercel/next.js/blob/canary/errors/template.txt).

### Improving Documentation

- [x] Run `pnpm prettier-fix` to fix formatting issues before opening
the PR.
- [x] Read the Docs Contribution Guide to ensure your contribution
follows the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

Co-authored-by: Delba de Oliveira <[email protected]>
## Summary
Add `bash filename="Terminal"` to improve readability.

### Improving Documentation

- [x] Run `pnpm prettier-fix` to fix formatting issues before opening
the PR.
- [x] Read the Docs Contribution Guide to ensure your contribution
follows the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

Co-authored-by: Delba de Oliveira <[email protected]>
## Description
Replace `let` with `const` in [No async client component error
page](https://nextjs.org/docs/messages/no-async-client-component)
example.
Since in these examples, it doesn't need to be a `let`, as it doesn't do
any reassignment.

### Improving Documentation

- [x] Run `pnpm prettier-fix` to fix formatting issues before opening
the PR.
- [x] Read the Docs Contribution Guide to ensure your contribution
follows the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

Co-authored-by: Delba de Oliveira <[email protected]>
## Description
[next-request-in-use-cache error
page](https://nextjs.org/docs/messages/next-request-in-use-cache) uses
-/+ icons in code block.
However, it doesn't work correctly because it's not diff code block.
Then add highlight instead of that.

### Improving Documentation

- [x] Run `pnpm prettier-fix` to fix formatting issues before opening
the PR.
- [x] Read the Docs Contribution Guide to ensure your contribution
follows the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

Co-authored-by: Delba de Oliveira <[email protected]>
### What?

* never make 0 sized spans
* support multiple search queries
### Why?

Example `blog-starter` was using `params` without awaiting.
In order to take advantage of the existing plumbing of the postponed state, we're injecting the resume data cache into the postponed state itself, as the data is always co-located. This ensures that stale resumes can utilize the data out of the box without any additional infrastructure configuration.
At build time we merge dynamic params (e.g. `[lang]`) with search params
into the same object used as part of the pages/app renders. This is
semantically incorrect and has led to some unexpected downstream
behavior (such as dynamic params showing up as cache keys in flight data
for app router). Pages router seems to rely on this behavior (for ex,
`useRouter().query` combines both dynamic params with search params).

This avoids mutating `query` and instead combines them specifically in
the pages export case. This also fixes an inconsistency in where we're
reading params in pages render, specifically `getStaticProps` now reads
params from `params` rather than `query` to align with
`getServerSideProps` and not rely on the fact that we happen to merge
them.
<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

### What?

1. Document incompatibility of Turbopack with MDX remark and rehype
plugins
2. Improve casing and prose
3. Add link to `remark-gfm`

### Why?

It's confusing to users that plugins don't work with Turbopack (because
`mdxRs` is required, which [does not support
plugins](https://github.com/wooorm/mdxjs-rs#:~:text=This%20project%20does%20not%20yet%20support%20plugins.)):

- https://bsky.app/profile/matthamlin.bsky.social/post/3la2lzzuprj22

### How?

Edit docs page

Co-authored-by: JJ Kasper <[email protected]>
…idation reason (#72296)

Setting the environment variable when building (`next dev`)

```
NEXT_TURBOPACK_INCLUDE_UPDATE_REASONS=1
```

causes `[Update Reasons]` messages to be logged to the terminal console (not in the browser) when a file is changed:

![Screenshot 2024-11-04 at 4.31.16 PM.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/HAZVitxRNnZz8QMiPn4a/1af5a4f0-cfba-4ca8-b188-d6be2064b0dc.png)

These messages will not appear if no reason was recorded when the invalidation happened, so this provided on a best-effort basis.

**Why?** The hope is that this can help debug cases where users get stuck in a fast refresh update loop: https://vercel.slack.com/archives/C03S8ED1DKM/p1730737898652749

Closes PACK-3374
kdy1 and others added 30 commits November 14, 2024 22:47
)

### What?

Merge a dependency into the chunk if the current node is the only dependant of the dependency.

i.e. Optimize for cases where only one **dependant** exists for a specific node.

### Why?

If a node has a single incoming node (let's call it `source`), it means that it will always be included if the source node is included and not if the source node is not included. So the source and node act as a group, and this PR makes them a real group to reduce number of module fragments.

Before:
<img width="1039" alt="image" src="https://github.com/user-attachments/assets/24321064-fe34-45c6-bf02-81e482816741">
After:
<img width="1039" alt="image" src="https://github.com/user-attachments/assets/4709a1f9-097c-4771-b554-043c01c980db">

### benches/heavy-npm-deps

- 14862 modules => 9263 modules

- With tree shaking on canary: `144.13s user 14.13s system 910% cpu 17.373 total`
- With tree shaking and this PR: `124.60s user 12.41s system 878% cpu 15.601 total`
- Without tree shaking: `47.41s user 4.75s system 553% cpu 9.416 total`


### How?

Closes PACK-3441
Since prefetches include only static data, and are generated during the
server prerender, we can actually lift all the tokens to the route tree
prefetch. It's only once we start supporting dynamic conditional routes
that the tokens will need to provided by the layout.

This is nice because it means we can parallelize the segment prefetches
after all.
Discussion with @unstubbable was that he didn't know if he should use
`String` or `RcStr`. This should clarify what people should use, why,
and how to construct the `RcStr`.

![Screenshot 2024-11-14 at 15-14-48 RcStr in turbo_tasks -
Rust.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/HAZVitxRNnZz8QMiPn4a/ad9aeb14-16f5-49d4-8433-fb25d4d1214c.png)
…ks-fetch (#72526)

Noticed this mess while working with @samcx on #72442 where we had to run:

```
cargo nextest r --features next-core/native-tls -p next-core
```

This PR makes `turbo-tasks-fetch` responsible for setting the `reqwest` TLS features, rather than trying to pass that feature flag down.

After this PR, running tests on `next-core` works with just:

```
cargo nextest r -p next-core
```

## Why?

- The feature flag logic was getting duplicated in multiple top-level targets.
- Packages that depended on `turbo-tasks-fetch` (directly *or transitively*) had to specify a default feature and make sure that default feature was disabled in the workspace's dependency list so that `cargo test -p package-name` would work
- ... or you had to specify a flag when compiling, like `--features next-core/native-tls`.

Technically this change gives us less flexibility. We can't produce multiple binaries for the same platform target with different TLS stacks. But I don't think we would've ever used that flexibility anyways.
`dynamic_embed_contents` is off by default, so i guess we missed some
type errors when refactoring stuff to `RcStr`
### What?

Currently when there are multiple instances of the same error on different lines of the same source file it shows a random one of these errors.

This fixes that by sorting the issues also based on the source.
### What?

Follow-up for #72564

### Why?

We need to refactor this for future works.

### How?

---------

Co-authored-by: Benjamin Woodruff <[email protected]>
… types (#72823)

`Vc<T>` had a type bound that `T: Send + ?Sized`. The general recommendation is to remove all type bounds from the struct that aren't necessary:

- https://rust-lang.github.io/api-guidelines/future-proofing.html#c-struct-bounds
- rust-lang/api-guidelines#6
- rust-lang/rust-clippy#1689

The reasoning is that type bounds on structs are "infectious". Any generic function, trait, or struct referring to `Vc<T>` was required to add `T: Send` bounds.

*Sidenote: The [`implied_bounds` feature](https://rust-lang.github.io/rfcs/2089-implied-bounds.html) might mitigate some of this if ever stabilized.*

Removing the `T: Send` type bound from `Vc<T>` means that there's less places where we need to specify it.

This pattern can be seen in many parts of the stdlib. For example, `Arc<T>` doesn't require that `T: Send + Sync`, though in reality to do anything useful with it, you need `T: Send + Sync`.

## Is this safe?

Yes, bounds are checked during cell construction, and the lower-level APIs used for creating cells require `Send + Sync`, so this would be hard to mess up without explicitly unsafe code.

Also, `Vc<T>` also technically requires that `T: Sync`, but that wasn't enforced in the struct definition (only during cell construction). We did just fine without that bound on the struct.

## Why are you leaving `?Sized`?

There's an implicit `Sized` bound on type parameters in struct definitions unless explicitly specified otherwise with `?Sized`.

Right now this bound isn't used. We use a box, e.g. `Vc<Box<dyn Foo>>`, but in the future we might be able to drop that intermediate box from the type signature. I'm leaving the `?Sized` bound in place (and adding it in a few places where it was missing) in hopes of that.
…2607)

### What?

Disable CSS Modules renaming for containers.

### Why?

We should match the behavior of webpack mode.

See: parcel-bundler/lightningcss#835

### How?

 - Closes PACK-3315
 - Closes #71233
### What?

Cleanup logic.

 - Remove duplicate imports.
 - Rename `exports` to `outputs` as it's more intuitive.
 - Do not track the `ix` of each exports, as it's not used.


### Why?


### How?
### What?

We need to notify the in progress cells when task has finished or cells are updated

Also fixes a off-by-one bug with max cell indicies when reading
### What?

The `data` variant for pages and the `rsc` variant for app router (for change notification purposes) causes some conflicting writes in the next-rs test case. This causes some instability since we are invalidating in a loop.
#72811)

This improves maintainability and readability by decluttering the server actions transforms implementation, dedupes a few error messages, and consistently uses periods and trailing newlines for all error messages.

The same approach is already used for the React Server Components transforms.
### Why?

We are renaming `revalidate(Tag|Path)` to `expire(Tag|Path)`.

Closes NDX-482

---------

Co-authored-by: Zack Tanner <[email protected]>
Hehe. `flightData` is either an array or a string. There's a loop that's
meant to iterate over `flightData` in the case where it's an array, but
we neglected to check whether it was an array or a string, and since
strings are iterable types, the type system did not complain.

The check that should have been before the loop was instead inside the
loop... and it happened to work anyway, because when you iterate over a
string, each item is a character, and characters have the type "string".

So the code works despite being nonsensical. In conclusion, lmfao.
This error doesn't point to any particularly meaningful line of code to
the user, we just use it to have a well-known error message for when a
feature should only be used in canary. This hides the stack trace so the
error is surfaced more clearly.
Updates the client PPR implementation to gracefully handle missing
CacheNodeSeedData when constructing a prerender tree. Currently this
never happens because we always block until we receive a server
response. However, in the new Segment Cache, we will not bother to wait
if a URL has not already been prefetched, since it's usually faster to
go straight to the dynamic request.
Wraps the `prefetch` API in a feature check for the Segment Cache, and
forwards the call to a stub for the new implementation.

Unlike the old implementation, the Segment Cache doesn't store its data
in the router reducer state; it writes into a global mutable cache. So
we don't need to dispatch a router action.

Since the Segment Cache isn't actually implemented yet, this effectively
disables prefetching when the experimental flag is enabled.

There's some validation that we do for prefetch URLs that I extracted
into a shared function. (For example, we only prefetch same-origin URLs,
and we don't prefetch anything in development.)
…72865)

In the Segment Cache implementation, I won't be using the main
fetchFromServerResponse function because the protocol for per-segment
prefetches is different. But there's a lot of logic (e.g. setting
headers, decoding the Flight stream, validating the response) that can
be shared. So I've extracted the shared bits into separate functions.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->
…72858)

The `RcStr` type doesn't use anything in `turbo-tasks`, so there's no reason it can't be a separate crate that doesn't depend on `turbo-tasks`.

This will let it be used in wasm swc plugins, where turbo-tasks does not currently compile.

Imports were automatically rewritten with:

```
# Fix imports:
sg run --lang rust --pattern 'use $CRATE::{$$$PRE, RcStr};' --rewrite 'use $CRATE::{$$$PRE}; use turbo_rcstr::RcStr;' --update-all
sg run --lang rust --pattern 'use $CRATE::{RcStr, $$$POST};' --rewrite 'use $CRATE::{$$$POST}; use turbo_rcstr::RcStr;' --update-all
sg run --lang rust --pattern 'use $CRATE::{$$$PRE, RcStr, $$$POST};' --rewrite 'use $CRATE::{$$$PRE, $$$POST}; use turbo_rcstr::RcStr;' --update-all
sg run --lang rust --pattern 'use $CRATE::RcStr;' --rewrite 'use turbo_rcstr::RcStr;' --update-all
cargo fmt

# Fix strings in comments:
fastmod crate::RcStr turbo_rcstr::RcStr
fastmod turbo_tasks::RcStr turbo_rcstr::RcStr
```

I had to manually fix the `Cargo.toml` files based on the compilation errors.
In the current navigation implementation, a partially dynamic navigation
always does two separate requests: one for the static data, and one for
the dynamic data. Typically the static data is prefetched before the
navigation begins, but even in the case where it is not, the current
implementation will still fetch it first. It then wait to send a dynamic
request until the first chunk is received from the prefetch response,
leading to an unfortunate request waterfall.

In the Segment Cache implementation, our plan is to never block a
navigation on prefetch data that isn't already populated in the cache.
Instead, in the case of a cache miss, we'll immediately start a dynamic
navigation and rely on the fact that the first thing the dynamic
response sends is the static PPR shell of the target page.

Because we'll always have this as a fallback behavior for cache misses,
it's a good starting point for the Segment Cache implementation. Then we
can start incrementally adding more and more features until we've
eventually reached/surpassed parity with the current implementation.

---

To avoid duplication of logic, I've chosen to model cache misses as a
special case of the normal static + dynamic flow. We can pretend that
the route tree returned by the dynamic request is, in fact, the result
of a prefetch. Then we use that same server response to write data into
the CacheNode tree. So it's the same flow as the "happy path", except we
use a single server response for both stages.
This breaks out the type overriding note from a small good to know
section to it's own section as it's fairly important and easy to miss
currently. Also the minimal note isn't getting picked up in our search
currently.

x-ref: [slack
thread](https://vercel.slack.com/archives/C0676QZBWKS/p1731602395153929)
)

### What?

Use `ResolvedVc` for struct fields in `turbopack-ecmascript` crate

### Why?

Follow-up of #72200,
#72382,
#72320

### How?

Closes PACK-3438

---------

Co-authored-by: Benjamin Woodruff <[email protected]>
### What?

Previously, we had a workaround while consuming the graph and converting them to `Module`s.

### Why?

Everything should be stored in the dependency graph because we are going to optimize it using graph node traversal.

### How?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.