Skip to content

Commit

Permalink
fix(config): only warn experimental feature when used (#37755)
Browse files Browse the repository at this point in the history
<!--
Thanks for opening a PR! Your contribution is much appreciated.
In order 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 that you're making:
-->

```js
// next.config.js
module.exports = {
  experimental: {
    legacyBrowsers: false,
    sharedPool: true,
    newNextLinkBehavior: false
  }
}
```

With the current implementation, using the config above will warn the usage of experimental features. However, those are the preset values that are defined in `defaultConfig` which means actually no experimental feature has been enabled at all.

The PR changes to only check if experimental features are actually enabled (have different values than the ones defined in `defaultConfig`).
  • Loading branch information
SukkaW committed Jun 18, 2022
1 parent af9d922 commit 5c88484
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 17 deletions.
53 changes: 45 additions & 8 deletions packages/next/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import { CONFIG_FILES, PHASE_DEVELOPMENT_SERVER } from '../shared/lib/constants'
import { execOnce } from '../shared/lib/utils'
import {
defaultConfig,
NextConfigComplete,
normalizeConfig,
ExperimentalConfig,
NextConfigComplete,
} from './config-shared'
import { loadWebpackHook } from './config-utils'
import {
Expand Down Expand Up @@ -43,6 +44,23 @@ const experimentalWarning = execOnce(
}
)

const missingExperimentalWarning = execOnce(
(configFileName: string, features: string[]) => {
const s = features.length > 1 ? 's' : ''
const dont = features.length > 1 ? 'do not' : 'does not'
const them = features.length > 1 ? 'them' : 'it'
Log.warn(
chalk.bold(
`You have defined experimental feature${s} (${features.join(
', '
)}) in ${configFileName} that ${dont} exist in this version of Next.js.`
)
)
Log.warn(`Please remove ${them} from your configuration.`)
console.warn()
}
)

function assignDefaults(userConfig: { [key: string]: any }) {
const configFileName = userConfig.configFileName
if (typeof userConfig.exportTrailingSlash !== 'undefined') {
Expand Down Expand Up @@ -77,13 +95,32 @@ function assignDefaults(userConfig: { [key: string]: any }) {
return currentConfig
}

if (
key === 'experimental' &&
value !== defaultConfig[key] &&
typeof value === 'object' &&
Object.keys(value).length > 0
) {
experimentalWarning(configFileName, Object.keys(value))
if (key === 'experimental' && typeof value === 'object') {
const enabledMissingExperiments: string[] = []
const enabledExperiments: (keyof ExperimentalConfig)[] = []

// defaultConfig.experimental is predefined and will never be undefined
// This is only a type guard for the typescript
if (defaultConfig.experimental) {
for (const featureName of Object.keys(
value
) as (keyof ExperimentalConfig)[]) {
if (!(featureName in defaultConfig.experimental)) {
enabledMissingExperiments.push(featureName)
} else if (
value[featureName] !== defaultConfig.experimental[featureName]
) {
enabledExperiments.push(featureName)
}
}
}

if (enabledMissingExperiments.length > 0) {
missingExperimentalWarning(configFileName, enabledMissingExperiments)
}
if (enabledExperiments.length > 0) {
experimentalWarning(configFileName, enabledExperiments)
}
}

if (key === 'distDir') {
Expand Down
79 changes: 70 additions & 9 deletions test/integration/config-experimental-warning/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ describe('Config Experimental Warning', () => {
module.exports = {
target: 'server',
experimental: {
something: true
newNextLinkBehavior: true
}
}
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have enabled experimental feature (something) in next.config.js.'
'You have enabled experimental feature (newNextLinkBehavior) in next.config.js.'
)
})

Expand All @@ -59,43 +59,104 @@ describe('Config Experimental Warning', () => {
module.exports = (phase) => ({
target: 'server',
experimental: {
something: true
newNextLinkBehavior: true
}
})
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have enabled experimental feature (something) in next.config.js.'
'You have enabled experimental feature (newNextLinkBehavior) in next.config.js.'
)
})

it('should not show warning with default value', async () => {
configFile.write(`
module.exports = (phase) => ({
target: 'server',
experimental: {
newNextLinkBehavior: false
}
})
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).not.toMatch(
'You have enabled experimental feature (newNextLinkBehavior) in next.config.js.'
)
})

it('should show warning with config from object with experimental and multiple keys', async () => {
configFile.write(`
module.exports = {
experimental: {
something: true,
another: 1,
newNextLinkBehavior: true,
legacyBrowsers: false,
}
}
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have enabled experimental features (something, another) in next.config.js.'
'You have enabled experimental features (newNextLinkBehavior, legacyBrowsers) in next.config.js.'
)
})

it('should show warning with next.config.mjs from object with experimental', async () => {
configFileMjs.write(`
const config = {
experimental: {
something: true,
newNextLinkBehavior: true,
}
}
export default config
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have enabled experimental feature (something) in next.config.mjs.'
'You have enabled experimental feature (newNextLinkBehavior) in next.config.mjs.'
)
})

it('should show warning with next.config.js from object with non-exist experimental', async () => {
configFile.write(`
const config = {
experimental: {
foo: true
}
}
module.exports = config
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have defined experimental feature (foo) in next.config.js that does not exist in this version'
)
})

it('should show warning with next.config.mjs from object with non-exist experimental', async () => {
configFileMjs.write(`
const config = {
experimental: {
foo: true
}
}
export default config
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have defined experimental feature (foo) in next.config.mjs that does not exist in this version'
)
})

it('should show warning with next.config.js from object with multiple non-exist experimental', async () => {
configFile.write(`
const config = {
experimental: {
foo: true,
bar: false
}
}
module.exports = config
`)
const { stderr } = await nextBuild(appDir, [], { stderr: true })
expect(stderr).toMatch(
'You have defined experimental features (foo, bar) in next.config.js that do not exist in this version'
)
})
})

0 comments on commit 5c88484

Please sign in to comment.