Skip to content

fix: should not warn depredated config if next config file does not exist #82593

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

Closed
huozhi wants to merge 7 commits into from
Closed

fix: should not warn depredated config if next config file does not exist #82593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3abd1e9
fix: should not warn depredated config if next config file is not def…
huozhi Aug 13, 2025
e8a27f0
fix test
huozhi Aug 13, 2025
8167bd4
log all warnings against userConfig
huozhi Aug 13, 2025
1c92717
more userConfig
huozhi Aug 13, 2025
e3341c2
fix test fixture
huozhi Aug 13, 2025
23424cc
revert
huozhi Aug 13, 2025
b68f0bb
Update test/unit/warn-removed-experimental-config.test.ts
huozhi Aug 13, 2025
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
2 changes: 2 additions & 0 deletions packages/next/src/server/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ describe('loadConfig', () => {
dynamicIO: true,
cacheComponents: false,
},
configFile: '/project/next.config.js',
},
silent: true,
})
Expand All @@ -232,6 +233,7 @@ describe('loadConfig', () => {
experimental: {
dynamicIO: true,
},
configFile: '/project/next.config.js',
},
silent: false,
})
Expand Down
46 changes: 26 additions & 20 deletions packages/next/src/server/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ export function warnOptionHasBeenDeprecated(
break
}
}
if (found) {

if (found && config.configFile) {
Copy link
Member

@ztanner ztanner Aug 13, 2025

Choose a reason for hiding this comment

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

we're adding a new property to NextConfig? I noticed there's also configFileName that we pass around.

I'm not crazy about adding new properties to the config object. I feel like the correct fix is to call these warning functions only when we have a user config, not some merged/default config.

Copy link
Member Author

@huozhi huozhi Aug 13, 2025

Choose a reason for hiding this comment

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

We're not adding a new prop, this is always present in loaded config, the filename is from findUp.

Copy link
Member

@devjiwonchoi devjiwonchoi Aug 13, 2025

Choose a reason for hiding this comment

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

nit: since now we pass userConfig, no need to check if the config file exists?

Log.warnOnce(reason)
hasWarned = true
}
Expand Down Expand Up @@ -524,7 +525,7 @@ function assignDefaults(
}

warnCustomizedOption(
result,
userConfig,
'experimental.esmExternals',
true,
'experimental.esmExternals is not recommended to be modified as it may disrupt module resolution',
Expand All @@ -533,44 +534,49 @@ function assignDefaults(
)

warnOptionHasBeenDeprecated(
result,
userConfig,
'experimental.instrumentationHook',
`\`experimental.instrumentationHook\` is no longer needed, because \`instrumentation.js\` is available by default. You can remove it from ${configFileName}.`,
silent
)

warnOptionHasBeenDeprecated(
result,
userConfig,
'experimental.after',
`\`experimental.after\` is no longer needed, because \`after\` is available by default. You can remove it from ${configFileName}.`,
silent
)

warnOptionHasBeenDeprecated(
result,
userConfig,
'devIndicators.appIsrStatus',
`\`devIndicators.appIsrStatus\` is deprecated and no longer configurable. Please remove it from ${configFileName}.`,
silent
)

warnOptionHasBeenDeprecated(
result,
userConfig,
'devIndicators.buildActivity',
`\`devIndicators.buildActivity\` is deprecated and no longer configurable. Please remove it from ${configFileName}.`,
silent
)

const hasWarnedBuildActivityPosition = warnOptionHasBeenDeprecated(
result,
userConfig,
'devIndicators.buildActivityPosition',
`\`devIndicators.buildActivityPosition\` has been renamed to \`devIndicators.position\`. Please update your ${configFileName} file accordingly.`,
silent
)
if (
hasWarnedBuildActivityPosition &&
result.devIndicators !== false &&
Copy link
Contributor

@vercel vercel bot Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The buildActivityPosition deprecation logic has inconsistent references between userConfig and result, which could cause runtime errors when accessing properties that may not exist.

View Details
📝 Patch Details 
diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts
index 3c4ff78cb7..6a74d5e503 100644
--- a/packages/next/src/server/config.ts
+++ b/packages/next/src/server/config.ts
@@ -569,14 +569,12 @@ function assignDefaults(
   )
   if (
     hasWarnedBuildActivityPosition &&
-    userConfig.devIndicators !== false &&
-    userConfig.devIndicators &&
-    'buildActivityPosition' in userConfig.devIndicators &&
-    userConfig.devIndicators.position &&
-    userConfig.devIndicators.buildActivityPosition !==
-      userConfig.devIndicators.position &&
+    result.devIndicators !== false &&
     result.devIndicators &&
-    'buildActivityPosition' in result.devIndicators
+    'buildActivityPosition' in result.devIndicators &&
+    result.devIndicators.position &&
+    result.devIndicators.buildActivityPosition !==
+      result.devIndicators.position
   ) {
     Log.warnOnce(
       `The \`devIndicators\` option \`buildActivityPosition\` ("${result.devIndicators.buildActivityPosition}") conflicts with \`position\` ("${result.devIndicators.position}"). Using \`buildActivityPosition\` ("${result.devIndicators.buildActivityPosition}") for backward compatibility.`

Analysis

In the buildActivityPosition logic, the code checks for properties on userConfig (lines 572-577) but then tries to access and use the same properties on result (lines 581-584). This creates several issues:

  1. Property existence mismatch: The code checks userConfig.devIndicators.position exists (line 575), but then uses result.devIndicators.position in the warning message (line 582). If position is a default value only present in result and not in the original userConfig, the condition will fail incorrectly.

  2. Type safety violation: The condition userConfig.devIndicators.position && assumes this property exists, but it may be undefined in the user's original config while being present in the merged result.

  3. Logical inconsistency: The warning message and final assignment use result.devIndicators.buildActivityPosition and result.devIndicators.position, but the conditions that trigger this logic are based on userConfig properties.

This could lead to the warning logic not triggering when it should, or potentially accessing undefined properties if the object structures differ between userConfig and result.

Recommendation

Make the property references consistent. Since the warning message and final assignment use result properties, the conditions should also check result properties:

if (
  hasWarnedBuildActivityPosition &&
  result.devIndicators !== false &&
  result.devIndicators &&
  'buildActivityPosition' in result.devIndicators &&
  result.devIndicators.position &&
  result.devIndicators.buildActivityPosition !== result.devIndicators.position
) {

Alternatively, if the intent is to check the user's original config, then use userConfig consistently throughout, but ensure the warning message and assignments are safe.

Copy link
Member Author

@huozhi huozhi Aug 13, 2025

Choose a reason for hiding this comment

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

moved lower, we should check against userConfig. Still keeping cond is only to make ts happy

'buildActivityPosition' in result.devIndicators &&
result.devIndicators.buildActivityPosition !== result.devIndicators.position
userConfig.devIndicators !== false &&
userConfig.devIndicators &&
'buildActivityPosition' in userConfig.devIndicators &&
userConfig.devIndicators.position &&
userConfig.devIndicators.buildActivityPosition !==
userConfig.devIndicators.position &&
result.devIndicators &&
'buildActivityPosition' in result.devIndicators
) {
Log.warnOnce(
`The \`devIndicators\` option \`buildActivityPosition\` ("${result.devIndicators.buildActivityPosition}") conflicts with \`position\` ("${result.devIndicators.position}"). Using \`buildActivityPosition\` ("${result.devIndicators.buildActivityPosition}") for backward compatibility.`
Expand All @@ -579,28 +585,28 @@ function assignDefaults(
}

warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'bundlePagesExternals',
'bundlePagesRouterDependencies',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'serverComponentsExternalPackages',
'serverExternalPackages',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'relay',
'compiler.relay',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'styledComponents',
'compiler.styledComponents',
configFileName,
Expand All @@ -614,42 +620,42 @@ function assignDefaults(
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'reactRemoveProperties',
'compiler.reactRemoveProperties',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'removeConsole',
'compiler.removeConsole',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'swrDelta',
'expireTime',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'outputFileTracingRoot',
'outputFileTracingRoot',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'outputFileTracingIncludes',
'outputFileTracingIncludes',
configFileName,
silent
)
warnOptionHasBeenMovedOutOfExperimental(
result,
userConfig,
'outputFileTracingExcludes',
'outputFileTracingExcludes',
configFileName,
Expand Down
15 changes: 8 additions & 7 deletions test/integration/build-indicator/test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import fs from 'fs-extra'
import { join } from 'path'
import webdriver from 'next-webdriver'
import { findPort, launchApp, killApp, waitFor, check } from 'next-test-utils'
import { findPort, launchApp, killApp, waitFor, retry } from 'next-test-utils'
import stripAnsi from 'strip-ansi'

const appDir = join(__dirname, '..')
Expand Down Expand Up @@ -33,7 +33,7 @@ describe('Build Activity Indicator', () => {
`
module.exports = {
devIndicators: {
buildActivityPosition: 'ttop-leff'
buildActivityPosition: 'top-leff'
}
}
`
Expand All @@ -47,12 +47,13 @@ describe('Build Activity Indicator', () => {
})
await fs.remove(configPath)

await check(
() => stripAnsi(stderr),
new RegExp(
`Invalid "devIndicator.position" provided, expected one of top-left, top-right, bottom-left, bottom-right, received ttop-leff`
await retry(() => {
Copy link
Member Author

@huozhi huozhi Aug 13, 2025

Choose a reason for hiding this comment

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

This test was wrong before, there's no devIndicator.position defined in user config

Copy link
Member

@devjiwonchoi devjiwonchoi Aug 13, 2025

Choose a reason for hiding this comment

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

There's devIndicators.position, which replaced deprecated devIndicators.buildActivityPosition.

next.js/packages/next/src/server/config-shared.ts

Lines 1121 to 1137 in ea5b03b

/**
* Position of "building..." indicator in browser
* @default "bottom-right"
* @deprecated Renamed as `position`.
*/
buildActivityPosition?:
| 'top-left'
| 'top-right'
| 'bottom-left'
| 'bottom-right'
/**
* Position of the development tools indicator in the browser window.
* @default "bottom-left"
* */
position?: 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right'
}

Copy link
Member Author

@huozhi huozhi Aug 13, 2025

Choose a reason for hiding this comment

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

Based on the user input we should warn buildActivityPosition is invalid not saying position otherwise user will get confused where they define position in next.config.js. It's not accurate to warn the new option if it doesn't exist on user land

Copy link
Member

@devjiwonchoi devjiwonchoi Aug 13, 2025

Choose a reason for hiding this comment

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

I see what you mean 👍

const cleanStderr = stripAnsi(stderr)
// buildActivityPosition is deprecated, but we should warn it's not a valid option
expect(cleanStderr).toContain(
`Invalid input at "devIndicators.buildActivityPosition"`
)
)
})

if (app) {
await killApp(app)
Expand Down
6 changes: 2 additions & 4 deletions test/production/pnpm-support/app-multi-page/next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ const path = require('path')

module.exports = {
output: 'standalone',
experimental: {
// pnpm virtual-store-dir is outside the app directory
outputFileTracingRoot: path.resolve(__dirname, '../'),
},
// pnpm virtual-store-dir is outside the app directory
outputFileTracingRoot: path.resolve(__dirname, '../'),
}
39 changes: 38 additions & 1 deletion test/unit/warn-removed-experimental-config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
warnOptionHasBeenMovedOutOfExperimental,
warnOptionHasBeenDeprecated,
NextConfig,
} from 'next/dist/server/config'
import stripAnsi from 'strip-ansi'

Expand Down Expand Up @@ -139,9 +140,12 @@ describe('warnOptionHasBeenMovedOutOfExperimental', () => {

describe('warnOptionHasBeenDeprecated', () => {
let spy: jest.SpyInstance
beforeAll(() => {
beforeEach(() => {
spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
})
afterEach(() => {
spy.mockRestore()
})

it('should warn experimental.appDir has been deprecated', () => {
const config = {
Expand All @@ -157,4 +161,37 @@ describe('warnOptionHasBeenDeprecated', () => {
)
expect(spy).toHaveBeenCalled()
})

it('should not warn when config file is not defined', () => {
// When config file is not defined, the configFileName is default value,
// the configFile is undefined.
const config: NextConfig = {
configFile: undefined,
configFileName: 'next.config.js',
}

warnOptionHasBeenDeprecated(
config,
'experimental.appDir',
'experimental.appDir has been removed',
false
)

expect(spy).not.toHaveBeenCalled()
})

it('should not warn when config key does not match', () => {
const config = {
badAssetPrefixKey: '/bar',
}

warnOptionHasBeenDeprecated(
config,
'assetPrefix',
'assetPrefix is gone',
false
)

expect(spy).not.toHaveBeenCalled()
})
})
Loading