-
Notifications
You must be signed in to change notification settings - Fork 26.1k
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
Add compiler error for conflicting App Router and Pages Router in Turbopack #62531
Add compiler error for conflicting App Router and Pages Router in Turbopack #62531
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @timneutkens and the rest of your teammates on Graphite |
Stats from current PRDefault BuildGeneral
Client Bundles (main, webpack)
Legacy Client Bundles (polyfills)
Client Pages
Client Build Manifests
Rendered Page Sizes
Edge SSR bundle Size
Middleware size
Next Runtimes
build cache
Diff detailsDiff for middleware.jsDiff too large to display |
2337ece
to
e9068fb
Compare
Tests Passed |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@next/bundle-analyzer@14.1.1-canary.72, npm/@next/env@14.1.1-canary.72, npm/@next/eslint-plugin-next@14.1.1-canary.72, npm/@next/font@14.1.1-canary.72, npm/@next/mdx@14.1.1-canary.72, npm/@next/plugin-storybook@14.1.1-canary.72, npm/@next/polyfill-module@14.1.1-canary.72, npm/@next/polyfill-nomodule@14.1.1-canary.72, npm/@next/react-refresh-utils@14.1.1-canary.72, npm/@next/third-parties@14.1.1-canary.72, npm/create-next-app@14.1.1-canary.72, npm/eslint-config-next@14.1.1-canary.72, npm/next@14.1.1-canary.72 |
90770f8
to
aa300d7
Compare
46ad42b
to
4fa3fe0
Compare
b186df8
to
0d71e34
Compare
0d71e34
to
e8146d1
Compare
4fa3fe0
to
3f45e57
Compare
e8146d1
to
e7b50b9
Compare
3f45e57
to
5c0ea9d
Compare
.to_string(), | ||
) | ||
.cell(), | ||
severity: IssueSeverity::Error.cell(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically you could omit severity and description since they are constant. Just return the fixed value in the issue implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah wasn't sure if there was a reason this wasn't done in the other issues, copied this one from app_structure.rs DirectoryTreeIssue
## What? Fixes the check for Turbopack. Follow-up to #62531 which fixes the underlying issue but it wasn't checking the right value. <!-- 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 # --> Closes NEXT-2665
What?
Working on fixing
test/e2e/conflicting-app-page-error
, this adds a compiler error for the case where App Router routes conflict with Pages Router routes. It's not 1:1 the same error as in webpack because in the webpack version we hijacked the App Router resolving logic to assume there's a certain set of paths, where Turbopack has the full route to route tree resolving implementation which doesn't assume there's a single page that can be resolved.The tests are updated to reflect this change.
Closes NEXT-2592