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 #565

Merged
merged 921 commits into from
Jul 21, 2023
Merged

[pull] canary from vercel:canary #565

merged 921 commits into from
Jul 21, 2023

Conversation

pull[bot]
Copy link

@pull pull bot commented May 10, 2023

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 May 10, 2023 22:18
@pull pull bot added the ⤵️ pull label May 11, 2023
shuding and others added 28 commits July 3, 2023 10:34
As we moved to new CI runner in `#50436`, try to re-enable few flaky tests we disabled before
### Issue
When you edit .env* files, the pages under app dir that using env vars are not triggering hot reload

### Fix
Triggering serverComponentChanges hot reload action when we detect env or tsconfig related change. There's a time period that we need to wait before the compilation is finished. First we save a flag `reloadOnDone` if we need to reload when after compilation is done, by determining if `envChange` is `true` (we already know this in dev server). Then in the compilation hooks, we refresh RSC page once it's finished.

### Extra change 

since we're checking `event.action` in client hot reloader, and throwing error for unknown action, filter devPagesManifestUpdate out from unexpected action as it sometimes triggered as error in console. Introduced in #51516
Fixes NEXT-1261
See vercel/turborepo#5430

## Turbopack changes

* vercel/turborepo#5430 <!-- Alex Kirszenberg -
ReverseTopological -> AdjacencyMap -->
<!-- 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 #

-->

Co-authored-by: JJ Kasper <[email protected]>
Reverts #52032

Will reland after the release
#52147)

Currently we are validating the `experimental.serverActions` flag when creating the actual entries for Server Actions, this causes two problems. One is that syntax errors caught at compilation time are still shown, even if you don't have this flag enabled. Another problem is we still traverse the client graph to collect these action modules even if the flag isn't enabled.

This PR moves that check to be happening at compilation time, which addresses the two above but also brings the extra benefit of showing the exact span and module trace that errors:

<img width="974" alt="CleanShot 2023-07-03 at 20 26 34@2x" src="https://github.com/vercel/next.js/assets/3676859/1676b1f6-e205-4963-bce4-5b515a698e9c">
`antd` should be using `kebabCase`:
https://unpkg.com/browse/[email protected]/es/index.js

This addresses the problem below as I tested locally:

<img width="397" alt="CleanShot 2023-07-03 at 20 45 48@2x"
src="https://github.com/vercel/next.js/assets/3676859/73ae99b9-0f3b-49ef-9675-9f45322b708d">
Removes usage of custom runner on final step as it's not necessary
Ensures when the cancelled status occurs we don't mark it as passed.
### What?

Add all passing tests

### Why?

We enforced that they all run turbopack and these are passing.
If any of these tests would become red, that indicated a problem.
### What

Support `scroll={false}` for Link component in app router. This can be
used when you don't need to scroll back to top again when route url
changes. For instance hash query changes, if you want to keep the
scrolling as it is, you can use this option.

### How

Handling the `scroll` option in navigation reducer on client side.  

Fixes #50105
Fixes NEXT-1377

---------

Co-authored-by: Jiachi Liu <[email protected]>
)

## What?

Ensures the router instance passed for `next/navigation` in Pages Router is a stable reference. For App Router the router instance is already a stable reference, so making this one stable too would fix #18127.


## How?

Added `React.useMemo` around `adaptForAppRouterInstance`, previously it was recreated each render but that's not needed for this particular case. The searchParamsContext and pathnameContext do need a new value each render in order to ensure they get the latest value.


Fixes #18127
Fixes NEXT-1375
…#52061)

### What?
When there's a parallel route adjacent to a tree that has no page
component, it's treated as an invalid entry in `handleAppPing` during
dev HMR, which causes an infinite refresh cycle

### Why?
In #51413, an update was made to `next-app-loader` to support layout
files in parallel routes. Part of this change updated the parallel
segment matching logic to mark the parallel page entry as `[
'@parallel', [ 'page$' ] ]` rather than `[ '@parallel', 'page$' ]`.

This resulted in `handleAppPing` looking for the corresponding page
entry at `client@app@/@parallel/page$/page` (note the `PAGE_SEGMENT`
marker) rather than `client@app@/@parallel/page`, causing the path to be
marked invalid on HMR pings, and triggering an endless fastRefresh.

### How?
A simple patch to fix this would fix this is to update `getEntryKey` to
replace any `PAGE_SEGMENT`'s that leak into the entry which I did in
59a972f.

The other option that's currently implemented here is to only insert
PAGE_SEGMENT as an array in the scenario where there isn't a page
adjacent to the parallel segment. This is to ensure that the
`parallelKey` is `children` rather than the `@parallel` slot when in
[`createSubtreePropsFromSegmentPath`](https://github.com/vercel/next.js/blob/59a972f53339cf6e444e3bf5be45bf115a24c31a/packages/next/src/build/webpack/loaders/next-app-loader.ts#L298).
This seems to not cause any regressions with the issue being fixed in
51413, and also solves this case, but I'm just not 100% sure if this
might break another scenario that I'm not thinking of.

Closes NEXT-1337
Fixes #51951
### What?

These test cases are too unreliable

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
### What?

I forgot to update the `needs` in the workflow
…e runtime (#52009)

As discussed in https://vercel.slack.com/archives/C03ENM5HB4K/p1687999628589119 and #51910, it makes sense to have a known list for packages (mostly polyfills) that we know are having dynamic code (`eval`, `new Function`) but are safe to run in the Edge Runtime because that dynamic code will never be executed.
huozhi and others added 22 commits July 19, 2023 16:15
As we're using nodejs with latest version of 18.x on CI, there's a new behavior change of `url.fileURLToPath` that didn't join the paths as expected

locally with node 16.x or 18.16
```
/private/var/folders/mh/y8kwzkls6v3_w3_k6q384cw80000gn/T/next-install-b033b516612809c6fb0a0de77c6e50f7d7f8af34dfd3d01812b99345352dc992/node_modules/.pnpm/file+..+next-repo-7e52b07043e986127273a2d951e5c412b0dd45fb24eb34001bd372e2afff79db+packages+n_vyqu6i4c4i3efp5pqsaeaoe5s4/node_modules/next/dist/compiled/@vercel/og/noto-sans-v27-latin-regular.ttf
```

in the actions runner with node 18.17 it's
```
/tmp/next-install-8efaa47c1546bed07990d8f130decceb6536e1a36146e7885cb68e3c3dea21f7/node_modules/.pnpm/file+..+next-repo-9679355e05341947e7aa0b42c994e3ac_krcuriy4thl7zawouf7sswlrgy/node_modules/next/dist/compiled/@vercel/og/index.node.js/../noto-sans-v27-latin-regular.ttf
```

You can see that `index.node.js` is still present in the path as a folder which is unexpected
```
index.node.js/../
```

x-ref: https://github.com/vercel/next.js/actions/runs/5600417148/jobs/10242827739?pr=52790
### What?

* adds `Project.update` to update project options
* fix manifest paths to be under `server`
* pass `env` into project
* handle and expose issues in all methods
* expose server paths in WrittenEndpoint
…2902)

Fixes #52853

Lacking 'color' attribute in IconDescriptor  Metadata

```diff
export type IconDescriptor = {
  url: string | URL
  type?: string
  sizes?: string
+ color?: string          // added the color attribute 
  /** defaults to rel="icon" unless superseded by Icons map */
  rel?: string
  media?: string
  /**
   * @see https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/fetchPriority
   */
  fetchPriority?: 'high' | 'low' | 'auto'
}
```


https://github.com/vercel/next.js/blob/v13.4.11-canary.0/packages/next/src/lib/metadata/types/metadata-types.ts#L99
Fixes a bug where the edge runtime didn't support `basePath` with Custom App Routes.

- Fixes #49661
### What?
- Visiting a page in the app router without a proper component export doesn't show the dev overlay, but logs errors to the console
- When it does show the error overlay (e.g. during an HMR event), the error message was sharing the module code itself rather than the component path, making it hard to debug

### Why?
`createComponentTree` can throw these errors before the AppRouter tree is mounted, leaving the errors uncaught by the dev overlay.

### How?
This wraps the server root in the `ReactDevOverlay` when in dev mode with a minimal "HMR" for when the server component is edited (to reload the page).

Closes NEXT-308
We were preventing smooth scrolling to avoid jarring UX when `scroll-behavior: smooth` was set and the user navigates to another route ([PR](#40642) and [related comment](#51721 (comment))). 

This updates both pages & app router to restore smooth scroll functionality if the only the route hash changes.

Fixes #51721
Closes NEXT-1406
### What?

adds edge pages, apis, app pages and app routes to next.rs api

### Why?

### How?
### What?

Adds a GitHub Action that automatically marks unhelpful comments as "off-topic".

You can test it by adding off-topic comments to this issue: balazsorban44#21

https://github.com/balazsorban44/next.js/blob/17e3ec30689a2323161260c9b03bb7f51ed008f9/.github/actions/minus-one/src/index.mjs#L6

### Why?

A common problem is when an issue thread receives "+1" comments, it usually encourages others to do the same. This makes the thread harder and harder for maintainers to read, without adding value to the discussion.

### How?

When an issue receives a comment, we compare it to a list that we determine as off-topic. If it matches, we hide the comment.

See: https://docs.github.com/en/graphql/reference/mutations#minimizecomment
Closes #52898

LangCode Missing zh-Hans and zh-Hant

| 'yo-NG'
  | 'zh-CN'
  | 'zh-Hans'   //added
  | 'zh-Hant'        //added
  | 'zh-HK'
  | 'zh-MO'
  | 'zh-SG'
  | 'zh-TW'
  | 'zu-ZA'


https://github.com/vercel/next.js/blob/f5272acbe5628164257ef8e584c8a469a4af5cb8/packages/next/src/lib/metadata/types/alternative-urls-types.ts#L419


Co-authored-by: Shu Ding <[email protected]>
…ng Named Imports" (#52932)

The correct filename is seen on PagesOnly. But on AppOnly, the filename is not correct. It should be "hello" instead of "ClientComponent".

line to change:
from:
import('../components/ClientComponent').then((mod) => mod.Hello)

to:
import('../components/hello').then((mod) => mod.Hello)


line to change:
from:
import('../components/ClientComponent').then((mod) => mod.Hello)

to:
import('../components/hello').then((mod) => mod.Hello)
### What?

Stop `with-supabase` template from throwing errors on build

### Why?

Dynamic Routes now fail the build, rather than console.logging an error

### How?

Export the following from any routes that use the `cookies` function

```
export const dynamic = "force-dynamic";
```

Install `encoding` as a dev dependency until this is fixed downstream
We have the logic to group the client compiler's entry names to make sure we generate one single manifest file for the page. This is complicated and requires a special step to "group" the entry names because a page can depend on a bunch of files from everywhere.

And currently, the normalization of "entryName → groupName" doesn't cover interception routes' conventions (`(.)`, `(..)` and `(...)`). This PR fixes that.

Closes #52862, closes #52681, closes #52958.
### What and why?

The word "publicly" should be spelled consistently across the documentation. It is spelled currently as "publically" in a few places.

### How?

Fixed the spelling!
### What

This PR changes the flow of not-found rendering process. 

### Why

`not-found.js` was rendered in two ways before:
* 1 is SSR rendering the not-found as 404
* 2 is triggering the error on RSC rendering then the error will be
preserved in inline flight data, on the client it will recover the error
and trigger the proper error boundary.

The solution has been through a jounery:
No top-level not found boundary -> introduce metadata API -> then we
create a top level root not found boundary -> then we delete it due to
duplicated rendering of root layout -> now this

So the solution before this PR is still having a root not found boundary
wrapped in the `AppRouter`, it's being used in a lot of places including
HMR. As we discovered it's doing duplicated rendering of root layout,
then we removed it and it started failing with rendering `not-found` but
missing root layout. In this PR we redesign the process.

### How

Now the rendering architecture looks like:

* For normal root not-found and certain level of not-found boundary
they're still covered by `LayoutRouter`
* For other error renderings including not-found
* Fully remove the top level not-found boundary, when it renders with
404 error it goes to render the fallback page
* During rendering the fallback page it will check if it should just
renders a 404 error page or render nothing and let the error from inline
flight data to trigger the error boundary

pseudo code
```
try {
  render AppRouter > PageComponent
} catch (err) {
  create ErrorComponent by determine err
  render AppRouter > ErrorComponent
}
```

In this way if the error is thrown from top-level like the page itself
or even from metadata, we can still catch them and render the proper
error page based on the error type.

The problematic is the HMR: introduces a new development mode meta tag
`<meta name="next-error">` to indicate it's 404 so that we don't do
refresh. This reverts the change brougt in #51637 as it will also has
the duplicated rendering problem for root layout if it's included in the
top level not found boundary.

Also fixes the root layout missing issue:

Fixes #52718
Fixes #52739

---------

Co-authored-by: Shu Ding <[email protected]>
This PR tries to address some feedback around prefetching, like in #49607, where they were some warnings because we were over prefetching.

The tweaks in this PR:
- if there are no loading boundary below, we don't prefetch the full page anymore. I made that change a while ago but I think it wasn't the original intent from @sebmarkbage. Fixing that now. So now, we will prefetch the page content until the nearest loading boundary, only if there is any.
- there's now a queue for limiting the number of concurrent prefetches. This is to not ruin the bandwidth for complex pages. Thanks @alvarlagerlof for that one.
- also, if the prefetch is in the queue when navigating, it will get bumped.
- bonus: fixes a bug where we were wrongly stripping headers in dev for static pages

Test plan:
<img width="976" alt="CleanShot 2023-07-20 at 17 53 43@2x" src="https://github.com/vercel/next.js/assets/11064311/2ea34419-c002-4aea-94df-57576e3ecb2e">
In the screenshot you can see that:
- only 5 requests at a time are authorised
- when I clicked on 15, it got prioritised in the queue (did not cancel the rest)
- the prefetch only included the content until the loading boundary




Co-authored-by: JJ Kasper <[email protected]>
ijjk and others added 6 commits July 20, 2023 15:10
We shouldn't be generating src files during the build script as these
can break publishing.
This breaks out routing handling from `next-server`, `next-dev-server`,
and `base-server` so that these are only handling the "render" work and
eventually these will be deleted completely in favor of the bundling
work being done.

The `router` process and separate `render` processes are still
maintained here although will be investigated further in follow-up to
see if we can reduce the need for these.

We are also changing the `require-cache` IPC to a single call instead of
call per entry to reduce overhead and also de-dupes handling for
starting the server between the standalone server and normal server.

To maintain support for existing turbopack route resolving this
implements the new route resolving in place of the existing
`route-resolver` until the new nextturbo API is fully landed.

After these initial changes we should continue to eliminate non-render
related code from `next-server`, `base-server`, and `next-dev-server`.
@pull pull bot merged commit f57eecd into MLH-Fellowship:canary Jul 21, 2023
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.