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.

Sign up for GitHub

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

Jump to bottom

refactor: added more strict app segment config validation #70480

Merged
merged 1 commit into from
Oct 4, 2024
Merged

refactor: added more strict app segment config validation #70480

merged 1 commit into from
Oct 4, 2024

Conversation

wyattjoh
Copy link
Member

@wyattjoh wyattjoh commented Sep 25, 2024 •
edited
Loading

This increases the strictness that Next.js uses while parsing configuration from applications as well as removing some legacy options (in preparation for Next.js 15).

  • Errors are now printing to the console while parsing app route segment configuration, improving the reliability of the parsed configuration
  • Breaking Pages and routes in App Router will no long support export const runtime = "experimental-edge", they will be required to switch to export const runtime = "edge". They were the same here anyways but this stabilizes it.
  • Enables the possibility of parsing the configuration values from client component files via direct AST traversal.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Sep 25, 2024
This was referenced Sep 25, 2024
Merged
Merged
@wyattjoh Graphite App
Copy link
Member Author

wyattjoh commented Sep 25, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @wyattjoh and the rest of your teammates on Graphite Graphite

@wyattjoh wyattjoh mentioned this pull request Sep 25, 2024
Merged
@ijjk
Copy link
Member

ijjk commented Sep 25, 2024 •
edited
Loading

Tests Passed

@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 1ebdb45 to c261a85 Compare September 25, 2024 18:17
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from c261a85 to 026cc27 Compare September 26, 2024 17:01
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 026cc27 to c552009 Compare September 26, 2024 18:24
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from c552009 to 998c585 Compare September 26, 2024 23:35
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 998c585 to 1ff36ff Compare September 26, 2024 23:46
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 1ff36ff to 351bbbc Compare September 27, 2024 00:02
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 351bbbc to 3686097 Compare September 27, 2024 18:23
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 3686097 to b8f312e Compare September 27, 2024 19:35
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from b8f312e to 3edece9 Compare September 29, 2024 16:30
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch 2 times, most recently from 7a1b186 to 6f319f5 Compare September 29, 2024 17:03
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 6f319f5 to 80164aa Compare September 29, 2024 17:09
@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch 11 times, most recently from cd8c4c3 to 6f99389 Compare October 3, 2024 02:39
ztanner
ztanner reviewed Oct 4, 2024
View reviewed changes
packages/next/src/server/dev/hot-reloader-webpack.ts

if (staticInfo?.type === PAGE_TYPES.PAGES) {
if (
staticInfo.config?.config?.amp === true ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come this is a doubly nested config?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pages/ supports the following:

export const config = { runtime: "edge" }

ztanner
ztanner reviewed Oct 4, 2024
View reviewed changes
packages/next/src/build/analysis/get-page-static-info.ts
params: GetPageStaticInfoParams
): Promise<PageStaticInfo> {
if (params.pageType === PAGE_TYPES.APP) {
return getAppPageStaticInfo(params)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

packages/next/src/build/entries.ts Outdated
@@ -148,45 +150,32 @@ export async function getStaticInfoIncludingLayouts({
dir = join(dir, '..')
}

// Reverse the layout files so we can use unshift to add them to the
// segments array.
layoutFiles.reverse()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this part is a little strange to me, could you clarify in the comment why we do this?

@wyattjoh wyattjoh force-pushed the refactor/app-segment-config-validation branch from 6f99389 to 64eae6e Compare October 4, 2024 19:32
@wyattjoh wyattjoh marked this pull request as ready for review October 4, 2024 19:32
@wyattjoh wyattjoh merged commit a20f750 into canary Oct 4, 2024
109 checks passed
@wyattjoh wyattjoh deleted the refactor/app-segment-config-validation branch October 4, 2024 20:03
@devjiwonchoi devjiwonchoi mentioned this pull request Oct 8, 2024
Merged
devjiwonchoi added a commit that referenced this pull request Oct 8, 2024
@devjiwonchoi
feat(next-codemod): add app-dir-runtime-config-experimental-edge (#…
f9d4066 
…70968)

### Why?

We had breaking change in App Router at
#70480 where it strictly checks
`runtime` route segment config. If it is set to `experimental-edge`,
will throw. Therefore we support a codemod to replace it to `edge`.
kdy1 pushed a commit that referenced this pull request Oct 10, 2024
@wyattjoh @kdy1
refactor: added more strict app segment config validation (#70480)
c395728 
<!-- 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 #

-->


This increases the strictness that Next.js uses while parsing
configuration from applications as well as removing some legacy options
(in preparation for Next.js 15).

- Errors are now printing to the console while parsing app route segment
configuration, improving the reliability of the parsed configuration
- **Breaking** Pages and routes in App Router will no long support
`export const runtime = "experimental-edge"`, they will be required to
switch to `export const runtime = "edge"`. They were the same here
anyways but this stabilizes it.
- Enables the possibility of parsing the configuration values from
client component files via direct AST traversal.
kdy1 pushed a commit that referenced this pull request Oct 10, 2024
@devjiwonchoi @kdy1
feat(next-codemod): add app-dir-runtime-config-experimental-edge (#…
881bd33 
…70968)

### Why?

We had breaking change in App Router at
#70480 where it strictly checks
`runtime` route segment config. If it is set to `experimental-edge`,
will throw. Therefore we support a codemod to replace it to `edge`.
@devjiwonchoi devjiwonchoi mentioned this pull request Oct 10, 2024
Merged
kdy1 pushed a commit that referenced this pull request Oct 10, 2024
@wyattjoh @kdy1
refactor: added more strict app segment config validation (#70480)
2254d42 
<!-- 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 #

-->


This increases the strictness that Next.js uses while parsing
configuration from applications as well as removing some legacy options
(in preparation for Next.js 15).

- Errors are now printing to the console while parsing app route segment
configuration, improving the reliability of the parsed configuration
- **Breaking** Pages and routes in App Router will no long support
`export const runtime = "experimental-edge"`, they will be required to
switch to `export const runtime = "edge"`. They were the same here
anyways but this stabilizes it.
- Enables the possibility of parsing the configuration values from
client component files via direct AST traversal.
kdy1 pushed a commit that referenced this pull request Oct 10, 2024
@devjiwonchoi @kdy1
feat(next-codemod): add app-dir-runtime-config-experimental-edge (#…
07712a0 
…70968)

### Why?

We had breaking change in App Router at
#70480 where it strictly checks
`runtime` route segment config. If it is set to `experimental-edge`,
will throw. Therefore we support a codemod to replace it to `edge`.
devjiwonchoi added a commit that referenced this pull request Oct 10, 2024
@devjiwonchoi @huozhi
chore(next-codemod): move app-dir-runtime-config-experimental-edge to…
f15a3f4 
… 13.1.2 (#71081)

### Why?

Although the breaking change for `export runtime = 'experimental-edge'`
for App Router was recent at
#70480, we did move from
experimental to `edge` at #44045,
and added log at #44563 (release
[v13.1.2](https://github.com/vercel/next.js/releases/tag/v13.1.2))

Therefore we lower the target codemod version to 13.1.2.

---------

Co-authored-by: Jiachi Liu <[email protected]>
eps1lon pushed a commit that referenced this pull request Oct 11, 2024
@devjiwonchoi @huozhi @eps1lon
chore(next-codemod): move app-dir-runtime-config-experimental-edge to…
f2ba4c0 
… 13.1.2 (#71081)

### Why?

Although the breaking change for `export runtime = 'experimental-edge'`
for App Router was recent at
#70480, we did move from
experimental to `edge` at #44045,
and added log at #44563 (release
[v13.1.2](https://github.com/vercel/next.js/releases/tag/v13.1.2))

Therefore we lower the target codemod version to 13.1.2.

---------

Co-authored-by: Jiachi Liu <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ztanner ztanner ztanner approved these changes

Assignees
No one assigned
Labels
created-by: Next.js team PRs by the Next.js team. locked tests type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wyattjoh @ijjk @ztanner