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

next-upgrade: Stop prompting for React 19 upgrade on pure App Router apps #71486

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Prompts for React 19 upgrade
Prompts for React 19 upgrade with a recommendation to do so
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-18-installed-mixed-router",
"scripts": {
"dev": "next dev"
"dev": "next dev --turbo"
},
"dependencies": {
"next": "14.3.0-canary.44",
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Prompts for React 19 upgrade
Upgrades to React 19 automatically
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-18-installed-pure-app-router",
"scripts": {
"dev": "next dev"
"dev": "next dev --turbo"
},
"dependencies": {
"next": "14.3.0-canary.44",
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Prompts for React 19 upgrade
Prompts for React 19 upgrade without any recommendation
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-18-installed-pure-pages-router",
"scripts": {
"dev": "next dev"
"dev": "next dev --turbo"
},
"dependencies": {
"next": "14.3.0-canary.44",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-19-installed-mixed-router",
"scripts": {
"dev": "next dev"
"dev": "next dev --turbo"
},
"dependencies": {
"next": "14.3.0-canary.45",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-19-installed-pure-app-router",
"scripts": {
"dev": "next dev"
"dev": "next dev --turbo"
},
"dependencies": {
"next": "14.3.0-canary.45",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-19-installed-pure-pages-router",
"scripts": {
"dev": "next dev"
"dev": "next dev --turbo"
},
"dependencies": {
"next": "14.3.0-canary.45",
Expand Down
31 changes: 29 additions & 2 deletions packages/next-codemod/bin/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ export async function runUpgrade(
console.log(` - React: v${installedReactVersion}`)
console.log(` - Next.js: v${installedNextVersion}`)
let shouldStayOnReact18 = false

const usesAppDir = isUsingAppDir(process.cwd())
const usesPagesDir = isUsingPagesDir(process.cwd())

const isPureAppRouter = usesAppDir && !usesPagesDir
const isMixedApp = usesPagesDir && usesAppDir
if (
// From release v14.3.0-canary.45, Next.js expects the React version to be 19.0.0-beta.0
// If the user is on a version higher than this but is still on React 18, we ask them
Expand All @@ -134,13 +140,21 @@ export async function runUpgrade(
// x-ref(PR): https://github.com/vercel/next.js/pull/65058
// x-ref(release): https://github.com/vercel/next.js/releases/tag/v14.3.0-canary.45
compareVersions(targetNextVersion, '14.3.0-canary.45') >= 0 &&
installedReactVersion.startsWith('18')
installedReactVersion.startsWith('18') &&
// Pure App Router always uses React 19
// The mixed case is tricky to handle from a types perspective.
// We'll recommend to upgrade in the prompt but users can decide to try 18.
!isPureAppRouter
) {
const shouldStayOnReact18Res = await prompts(
{
type: 'confirm',
name: 'shouldStayOnReact18',
message: `Are you using ${pc.underline('only the Pages Router')} (no App Router) and prefer to stay on React 18?`,
message:
`Do you prefer to stay on React 18?` +
(isMixedApp
? " Since you're using both pages/ and app/, we recommend upgrading React to use a consistent version throughout your app."
Copy link
Member

Choose a reason for hiding this comment

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

Staying on 18 is also consistent version, maybe inform (benefit of | risk of not) upgrading?

Copy link
Member Author

Choose a reason for hiding this comment

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

Staying on 18 is also consistent version

Not if you're using App Router (always uses React 19) which should the case in this branch.

We should link to the upgrade guide earlier in the CLI regardless. This is not the place to go into details about this. Or do we link to the upgrade guide at the end?

Copy link
Member

Choose a reason for hiding this comment

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

I see. No I think is good for how it is.

: ''),
initial: false,
active: 'Yes',
inactive: 'No',
Expand Down Expand Up @@ -371,6 +385,19 @@ function getInstalledReactVersion(): string {
}
}

function isUsingPagesDir(projectPath: string): boolean {
return (
fs.existsSync(path.resolve(projectPath, 'pages')) ||
fs.existsSync(path.resolve(projectPath, 'src/pages'))
)
}
function isUsingAppDir(projectPath: string): boolean {
return (
fs.existsSync(path.resolve(projectPath, 'app')) ||
fs.existsSync(path.resolve(projectPath, 'src/app'))
)
}
Comment on lines +388 to +399
Copy link
Member Author

Choose a reason for hiding this comment

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

False-positive for empty dirs or dirs without pages but this should be good enough.


/*
* Heuristics are used to determine whether to Turbopack is enabled or not and
* to determine how to update the dev script.
Expand Down
Loading