fix: [Next 16] remove 'next lint' #83135
Draft
+8
−1,080
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ijjk
added
created-by: Next.js team
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments:
packages/next/src/cli/next-build.ts (lines 97-97):
The build function call passes a lint parameter, but the build function no longer accepts this parameter after the ESLint functionality was removed.
View Details
Analysis
In packages/next/src/cli/next-build.ts on line 97, the build() function is being called with the lint parameter as the 5th argument. However, in packages/next/src/build/index.ts, the runLint = true parameter was removed from the function signature (originally the 4th parameter). This means the lint value is now being passed to what used to be the noMangling parameter, and all subsequent parameters are shifted, causing:
lint(boolean) passed tonoManglingparameter (expects boolean, but semantically wrong)!manglingpassed toappDirOnlyparameter (expects boolean)experimentalAppOnlypassed toisTurbopackparameter (expects boolean)isTurbopackpassed toexperimentalBuildModeparameter (expects string)experimentalBuildModepassed totraceUploadUrlparameter (expects string | undefined)traceUploadUrlnot passed at all
This will cause runtime errors or incorrect behavior during builds. The fix is to remove the lint parameter from the function call on line 97, since the build function no longer performs linting.
bgub
force-pushed
the
feat/remove-next-lint
branch
from
45c57e6 to
ff7f56a
Compare
bgub
force-pushed
the
feat/remove-next-lint
branch
from
ff7f56a to
504a8fe
Compare
bgub
force-pushed
the
graphite-base/83135
branch
from
8a594ed to
76f1a51
Compare
bgub
changed the base branch from
graphite-base/83135
to
feat/no-lint-in-build
bgub
force-pushed
the
feat/remove-next-lint
branch
from
504a8fe to
0c092d1
Compare
bgub
force-pushed
the
feat/no-lint-in-build
branch
from
76f1a51 to
0a90014
Compare
Failing test suitesCommit: 0c092d1
Expand output● eslint caching is enabled by default ● eslint caching is disabled with the --no-cache flag ● the default eslint cache lives in the user defined build directory ● the --cache-location flag allows the user to define a separate cache location ● the default eslint caching strategy is metadata ● cache with content strategy is different from the one with default strategy Read more about building and testing Next.js in contributing.md.
Expand output● Next Lint › First Time Setup › show a prompt to set up ESLint if no configuration detected ● Next Lint › First Time Setup › installs eslint and eslint-config-next as devDependencies if missing with yarn ● Next Lint › First Time Setup › installs eslint and eslint-config-next as devDependencies if missing with pnpm ● Next Lint › First Time Setup › installs eslint and eslint-config-next as devDependencies if missing with npm ● Next Lint › First Time Setup › creates .eslintrc.json file with a default configuration ● Next Lint › First Time Setup › creates .eslintrc.json file with a default app router configuration ● Next Lint › First Time Setup › shows a successful message when completed ● Next Lint › should generate next-env.d.ts before lint command ● Next Lint › shows warnings and errors ● Next Lint › verify options name and type with auto-generated help output ● Next Lint › base directories are linted by default ● Next Lint › shows warnings and errors with next/core-web-vitals config ● Next Lint › shows warnings and errors when extending plugin recommended config ● Next Lint › shows warnings and errors when extending plugin core-web-vitals config ● Next Lint › success message when no warnings or errors ● Next Lint › quiet flag suppresses warnings and only reports errors ● Next Lint › custom directories ● Next Lint › max warnings flag errors when warnings exceed threshold ● Next Lint › max warnings flag does not error when warnings do not exceed threshold ● Next Lint › format flag supports additional user-defined formats ● Next Lint › format flag supports async formatters ● Next Lint › file flag can selectively lint only a single file ● Next Lint › file flag can selectively lints multiple files ● Next Lint › format flag "json" creates a file respecting the chosen format ● Next Lint › lint files with cjs and mjs file extension Read more about building and testing Next.js in contributing.md.
Expand output● next-lint-eslint-formatter-compact › should format flag "compact" creates a file respecting the chosen format ● next-lint-eslint-formatter-compact › should show error message when the file path is a directory Read more about building and testing Next.js in contributing.md.
Expand output● Next Build › production mode › shows warnings and errors ● Next Build › production mode › base directories are linted by default during builds ● Next Build › production mode › custom directories ● Next Build › production mode › invalid older eslint version ● Next Build › production mode › empty directories do not fail the build ● Next Build › production mode › eslint ignored directories do not fail the build ● Next Build › production mode › missing Next.js plugin ● Next Build › production mode › eslint caching is enabled ● Next Build › production mode › eslint cache lives in the user defined build directory Read more about building and testing Next.js in contributing.md.
Expand output● eslint plugin deps › should work ● Test suite failed to run Read more about building and testing Next.js in contributing.md.
Expand output● no-eslint-warn-with-no-eslint-config › should warn with empty eslintrc ● no-eslint-warn-with-no-eslint-config › should warn with empty eslint config in package.json Read more about building and testing Next.js in contributing.md.
Expand output● Next Build › production mode › first time setup - ESLint v8 ● Next Build › production mode › first time setup - ESLint v9 ● Next Build › production mode › first time setup with TypeScript - ESLint v8 ● Next Build › production mode › first time setup with TypeScript - ESLint v9 Read more about building and testing Next.js in contributing.md.
Expand output● production - app dir - build output › should always log version first then the rest jobs Read more about building and testing Next.js in contributing.md.
Expand output● config telemetry › production mode › emits telemetry for lint during build ● config telemetry › production mode › emits telemetry for lint during build when '--no-lint' is specified ● config telemetry › production mode › emits telemetry for lint during build when 'ignoreDuringBuilds' is specified ● config telemetry › production mode › emits telemetry for Read more about building and testing Next.js in contributing.md.
Expand output● output: standalone with getStaticProps › should work correctly with output standalone Read more about building and testing Next.js in contributing.md. |
There was a problem hiding this comment.
Additional Comments:
test/lib/next-test-utils.ts (lines 486-492):
The nextLint function tries to execute the removed next lint command, which will cause all tests using this function to fail.
View Details
📝 Patch Details
diff --git a/test/development/next-lint-eslint-formatter-compact/index.test.ts b/test/development/next-lint-eslint-formatter-compact/index.test.ts
index 095b249fc0..190faaba3d 100644
--- a/test/development/next-lint-eslint-formatter-compact/index.test.ts
+++ b/test/development/next-lint-eslint-formatter-compact/index.test.ts
@@ -1,6 +1,6 @@
import { nextTestSetup } from 'e2e-utils'
import { readFileSync } from 'fs'
-import { nextLint } from 'next-test-utils'
+import { execSync } from 'child_process'
describe('next-lint-eslint-formatter-compact', () => {
const { next } = nextTestSetup({
@@ -10,14 +10,20 @@ describe('next-lint-eslint-formatter-compact', () => {
it('should format flag "compact" creates a file respecting the chosen format', async () => {
const filePath = `${next.testDir}/output/output.txt`
- const { stdout, stderr } = await nextLint(
- next.testDir,
- ['--format', 'compact', '--output-file', filePath],
- {
- stdout: true,
- stderr: true,
- }
- )
+ let stdout = ''
+ let stderr = ''
+ try {
+ stdout = execSync(
+ `npx eslint . --format compact --output-file ${filePath}`,
+ {
+ cwd: next.testDir,
+ encoding: 'utf8',
+ stdio: ['pipe', 'pipe', 'pipe']
+ }
+ )
+ } catch (error: any) {
+ stderr = error.stderr || error.message
+ }
const cliOutput = stdout + stderr
const fileOutput = readFileSync(filePath, 'utf8')
@@ -40,14 +46,20 @@ describe('next-lint-eslint-formatter-compact', () => {
it('should show error message when the file path is a directory', async () => {
const filePath = next.testDir
- const { stdout, stderr } = await nextLint(
- next.testDir,
- ['--format', 'compact', '--output-file', filePath],
- {
- stdout: true,
- stderr: true,
- }
- )
+ let stdout = ''
+ let stderr = ''
+ try {
+ stdout = execSync(
+ `npx eslint . --format compact --output-file ${filePath}`,
+ {
+ cwd: next.testDir,
+ encoding: 'utf8',
+ stdio: ['pipe', 'pipe', 'pipe']
+ }
+ )
+ } catch (error: any) {
+ stderr = error.stderr || error.message
+ }
const cliOutput = stdout + stderr
console.log({ cliOutput })
diff --git a/test/integration/eslint/test/lint-cache.test.js b/test/integration/eslint/test/lint-cache.test.js
index 2004b59de4..a53fb45e9d 100644
--- a/test/integration/eslint/test/lint-cache.test.js
+++ b/test/integration/eslint/test/lint-cache.test.js
@@ -1,13 +1,17 @@
+// NOTE: These tests are disabled because the `next lint` command has been deprecated
+// and removed from Next.js CLI in favor of using ESLint directly.
+// See: https://nextjs.org/blog/next-15-5#next-lint-deprecation
+
import fs from 'fs-extra'
import { join } from 'path'
-import { nextLint } from 'next-test-utils'
+// import { nextLint } from 'next-test-utils' // Function removed - next lint deprecated
const dirEslintCache = join(__dirname, '../eslint-cache')
const dirEslintCacheCustomDir = join(__dirname, '../eslint-cache-custom-dir')
-test('eslint caching is enabled by default', async () => {
+test.skip('eslint caching is enabled by default', async () => {
const cacheDir = join(dirEslintCache, '.next', 'cache')
await fs.remove(cacheDir)
@@ -19,7 +23,7 @@ test('eslint caching is enabled by default', async () => {
expect(cacheExists).toBe(true)
})
-test('eslint caching is disabled with the --no-cache flag', async () => {
+test.skip('eslint caching is disabled with the --no-cache flag', async () => {
const cacheDir = join(dirEslintCache, '.next', 'cache')
await fs.remove(cacheDir)
@@ -28,7 +32,7 @@ test('eslint caching is disabled with the --no-cache flag', async () => {
expect(fs.existsSync(join(cacheDir, 'eslint/'))).toBe(false)
})
-test('the default eslint cache lives in the user defined build directory', async () => {
+test.skip('the default eslint cache lives in the user defined build directory', async () => {
const oldCacheDir = join(dirEslintCacheCustomDir, '.next', 'cache')
const newCacheDir = join(dirEslintCacheCustomDir, 'build', 'cache')
@@ -45,7 +49,7 @@ test('the default eslint cache lives in the user defined build directory', async
expect(cacheExists).toBe(true)
})
-test('the --cache-location flag allows the user to define a separate cache location', async () => {
+test.skip('the --cache-location flag allows the user to define a separate cache location', async () => {
const cacheFile = join(dirEslintCache, '.eslintcache')
await fs.remove(cacheFile)
@@ -65,7 +69,7 @@ const getEslintCacheContent = async (cacheDir) => {
return await fs.readFile(cacheFile, 'utf8')
}
-test('the default eslint caching strategy is metadata', async () => {
+test.skip('the default eslint caching strategy is metadata', async () => {
const cacheDir = join(dirEslintCache, '.next', 'cache')
await fs.remove(cacheDir)
@@ -81,7 +85,7 @@ test('the default eslint caching strategy is metadata', async () => {
expect(metadataStrategyCache).toBe(defaultStrategyCache)
})
-test('cache with content strategy is different from the one with default strategy', async () => {
+test.skip('cache with content strategy is different from the one with default strategy', async () => {
const cacheDir = join(dirEslintCache, '.next', 'cache')
await fs.remove(cacheDir)
diff --git a/test/integration/eslint/test/next-lint.test.js b/test/integration/eslint/test/next-lint.test.js
index b88bd98ef3..db24d112fa 100644
--- a/test/integration/eslint/test/next-lint.test.js
+++ b/test/integration/eslint/test/next-lint.test.js
@@ -1,10 +1,14 @@
+// NOTE: These tests are disabled because the `next lint` command has been deprecated
+// and removed from Next.js CLI in favor of using ESLint directly.
+// See: https://nextjs.org/blog/next-15-5#next-lint-deprecation
+
import fs from 'fs-extra'
import os from 'os'
import { join } from 'path'
import findUp from 'next/dist/compiled/find-up'
-import { nextLint } from 'next-test-utils'
+// import { nextLint } from 'next-test-utils' // Function removed - next lint deprecated
const dirFirstTimeSetup = join(__dirname, '../first-time-setup')
const dirCustomConfig = join(__dirname, '../custom-config')
@@ -29,7 +33,7 @@ const mjsCjsLinting = join(__dirname, '../mjs-cjs-linting')
const dirTypescript = join(__dirname, '../with-typescript')
const formatterAsync = join(__dirname, '../formatter-async/format.js')
-describe('Next Lint', () => {
+describe.skip('Next Lint (DEPRECATED - command removed)', () => {
describe('First Time Setup ', () => {
async function nextLintTemp(setupCallback, isApp = false) {
const folder = join(os.tmpdir(), Math.random().toString(36).substring(2))
diff --git a/test/integration/telemetry/test/config.test.js b/test/integration/telemetry/test/config.test.js
index 95e75e96a5..907476da5f 100644
--- a/test/integration/telemetry/test/config.test.js
+++ b/test/integration/telemetry/test/config.test.js
@@ -5,7 +5,7 @@ import {
killApp,
launchApp,
nextBuild,
- nextLint,
+ // nextLint, // Function removed - next lint deprecated
waitFor,
} from 'next-test-utils'
import fs from 'fs-extra'
@@ -241,15 +241,16 @@ describe('config telemetry', () => {
})
})
- it('emits telemetry for `next lint`', async () => {
+ it.skip('emits telemetry for `next lint` (DEPRECATED)', async () => {
await fs.writeFile(
path.join(appDir, '.eslintrc'),
`{ "root": true, "extends": "next" }`
)
- const { stderr } = await nextLint(appDir, [], {
- stderr: true,
- env: { NEXT_TELEMETRY_DEBUG: 1 },
- })
+ // const { stderr } = await nextLint(appDir, [], { // Function removed - deprecated
+ // stderr: true,
+ // env: { NEXT_TELEMETRY_DEBUG: 1 },
+ // })
+ const stderr = '' // Disabled test - nextLint deprecated
await fs.remove(path.join(appDir, '.eslintrc'))
const event1 = /NEXT_LINT_CHECK_COMPLETED[\s\S]+?{([\s\S]+?)^}/m
diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts
index 9a4753a8d2..4dfa23d6ca 100644
--- a/test/lib/next-test-utils.ts
+++ b/test/lib/next-test-utils.ts
@@ -483,14 +483,6 @@ export function nextBuild(
return runNextCommand(['build', dir, ...args], opts)
}
-export function nextLint(
- dir: string,
- args: string[] = [],
- opts: NextOptions = {}
-) {
- return runNextCommand(['lint', dir, ...args], opts)
-}
-
export function nextTest(
dir: string,
args: string[] = [],
diff --git a/test/production/eslint/test/next-build-and-lint.test.ts b/test/production/eslint/test/next-build-and-lint.test.ts
index 279ad0bd33..75c784bebb 100644
--- a/test/production/eslint/test/next-build-and-lint.test.ts
+++ b/test/production/eslint/test/next-build-and-lint.test.ts
@@ -29,18 +29,19 @@ describe('Next Build', () => {
const nextBuildCommand = await next.build()
const buildOutput = nextBuildCommand.cliOutput
- expect(buildOutput).toContain(
- 'No ESLint configuration detected. Run next lint to begin setup'
- )
+ // expect(buildOutput).toContain(
+ // 'No ESLint configuration detected. Run next lint to begin setup'
+ // )
+ // Note: The build output may no longer mention `next lint` since the command is deprecated
- // TODO: Should we exit non-zero here if the config was created? Should we maybe even directly start linting?
+ // Verify that `next lint` command is no longer available (as it's been deprecated)
expect(() => {
execSync(`pnpm next lint --strict`, {
cwd: next.testDir,
encoding: 'utf8',
stdio: 'inherit',
})
- }).toThrow('Command failed: pnpm next lint --strict')
+ }).toThrow() // Should fail because command doesn't exist
const eslintConfigAfterSetupJSON = execSync(
`pnpm eslint --print-config pages/index.js`,
@@ -112,18 +113,19 @@ describe('Next Build', () => {
const nextBuildCommand = await next.build()
const buildOutput = nextBuildCommand.cliOutput
- expect(buildOutput).toContain(
- 'No ESLint configuration detected. Run next lint to begin setup'
- )
+ // expect(buildOutput).toContain(
+ // 'No ESLint configuration detected. Run next lint to begin setup'
+ // )
+ // Note: The build output may no longer mention `next lint` since the command is deprecated
- // TODO: Should we exit non-zero here if the config was created? Should we maybe even directly start linting?
+ // Verify that `next lint` command is no longer available (as it's been deprecated)
expect(() => {
execSync(`pnpm next lint --strict`, {
cwd: next.testDir,
encoding: 'utf8',
stdio: 'inherit',
})
- }).toThrow('Command failed: pnpm next lint --strict')
+ }).toThrow() // Should fail because command doesn't exist
const eslintConfigAfterSetupJSON = execSync(
// TODO(jiwon): remove `ESLINT_USE_FLAT_CONFIG=false` when we create the config for ESLint 9.
@@ -197,18 +199,19 @@ describe('Next Build', () => {
const nextBuildCommand = await next.build()
const buildOutput = nextBuildCommand.cliOutput
- expect(buildOutput).toContain(
- 'No ESLint configuration detected. Run next lint to begin setup'
- )
+ // expect(buildOutput).toContain(
+ // 'No ESLint configuration detected. Run next lint to begin setup'
+ // )
+ // Note: The build output may no longer mention `next lint` since the command is deprecated
- // TODO: Should we exit non-zero here if the config was created? Should we maybe even directly start linting?
+ // Verify that `next lint` command is no longer available (as it's been deprecated)
expect(() => {
execSync(`pnpm next lint --strict`, {
cwd: next.testDir,
encoding: 'utf8',
stdio: 'inherit',
})
- }).toThrow('Command failed: pnpm next lint --strict')
+ }).toThrow() // Should fail because command doesn't exist
const eslintConfigAfterSetupJSON = execSync(
`pnpm eslint --print-config pages/index.tsx`,
@@ -280,18 +283,19 @@ describe('Next Build', () => {
const nextBuildCommand = await next.build()
const buildOutput = nextBuildCommand.cliOutput
- expect(buildOutput).toContain(
- 'No ESLint configuration detected. Run next lint to begin setup'
- )
+ // expect(buildOutput).toContain(
+ // 'No ESLint configuration detected. Run next lint to begin setup'
+ // )
+ // Note: The build output may no longer mention `next lint` since the command is deprecated
- // TODO: Should we exit non-zero here if the config was created? Should we maybe even directly start linting?
+ // Verify that `next lint` command is no longer available (as it's been deprecated)
expect(() => {
execSync(`pnpm next lint --strict`, {
cwd: next.testDir,
encoding: 'utf8',
stdio: 'inherit',
})
- }).toThrow('Command failed: pnpm next lint --strict')
+ }).toThrow() // Should fail because command doesn't exist
const eslintConfigAfterSetupJSON = execSync(
// TODO(jiwon): remove `ESLINT_USE_FLAT_CONFIG=false` when we create the config for ESLint 9.
Analysis
The nextLint function in the test utilities calls runNextCommand(['lint', dir, ...args], opts) on line 491, but the lint command has been completely removed from the Next.js CLI in this changeset. This will cause runtime failures in multiple test files that depend on this function:
test/development/next-lint-eslint-formatter-compact/index.test.ts(2 test cases)test/integration/eslint/test/lint-cache.test.js(9 test cases)test/integration/eslint/test/next-lint.test.js(many test cases)
The function should either be removed entirely along with all the tests that use it, or the tests should be updated to use the ESLint CLI directly instead of the removed next lint command. Since the intent appears to be removing Next.js's built-in lint functionality in favor of native ESLint CLI usage, the most appropriate fix would be to remove this function and update the affected tests accordingly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR removes the ESLint integration from Next.js, including:
next lintcommandWhy?
As part of Next.js 16, we're removing the built-in ESLint integration to simplify the codebase and encourage users to use the ESLint CLI directly.
How?
next lintcommand from the CLI