From d12872246522bd20ee4d4f0cf281c3affbca36e2 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 14 Jun 2021 20:31:40 -0500 Subject: [PATCH] Add lint checking events (#26089) * Add lint checking events * remove extra log * Update check * Update version check --- packages/next/build/index.ts | 3 +- packages/next/cli/next-lint.ts | 37 ++++++++++-- packages/next/lib/eslint/customFormatter.ts | 60 +++++++++++++++---- packages/next/lib/eslint/runLintCheck.ts | 46 +++++++++++--- .../next/lib/has-necessary-dependencies.ts | 6 +- packages/next/lib/verifyAndLint.ts | 29 +++++++-- packages/next/lib/verifyTypeScriptSetup.ts | 4 +- packages/next/telemetry/events/build.ts | 21 +++++++ test/integration/telemetry/test/index.test.js | 45 ++++++++++++++ 9 files changed, 217 insertions(+), 34 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 2b7f07badffa6..edb867d66eea4 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -223,7 +223,8 @@ export default async function build( dir, lintDirs, config.experimental.cpus, - config.experimental.workerThreads + config.experimental.workerThreads, + telemetry ) }) } diff --git a/packages/next/cli/next-lint.ts b/packages/next/cli/next-lint.ts index 5c07725468732..04993ca8def78 100755 --- a/packages/next/cli/next-lint.ts +++ b/packages/next/cli/next-lint.ts @@ -8,6 +8,11 @@ import { cliCommand } from '../bin/next' import { ESLINT_DEFAULT_DIRS } from '../lib/constants' import { runLintCheck } from '../lib/eslint/runLintCheck' import { printAndExit } from '../server/lib/utils' +import { Telemetry } from '../telemetry/storage' +import loadConfig from '../next-server/server/config' +import { PHASE_PRODUCTION_BUILD } from '../next-server/lib/constants' +import { eventLintCheckCompleted } from '../telemetry/events' +import { CompileError } from '../lib/compile-error' const eslintOptions = (args: arg.Spec) => ({ overrideConfigFile: args['--config'] || null, @@ -135,11 +140,35 @@ const nextLint: cliCommand = (argv) => { }, [] ) - runLintCheck(baseDir, lintDirs, false, eslintOptions(args)) - .then((results) => { - if (results) { - console.log(results) + .then(async (lintResults) => { + const lintOutput = + typeof lintResults === 'string' ? lintResults : lintResults?.output + + if (typeof lintResults !== 'string' && lintResults?.eventInfo) { + const conf = await loadConfig(PHASE_PRODUCTION_BUILD, baseDir) + const telemetry = new Telemetry({ + distDir: join(baseDir, conf.distDir), + }) + telemetry.record( + eventLintCheckCompleted({ + ...lintResults.eventInfo, + buildLint: false, + }) + ) + await telemetry.flush() + } + + if ( + typeof lintResults !== 'string' && + lintResults?.isError && + lintOutput + ) { + throw new CompileError(lintOutput) + } + + if (lintOutput) { + console.log(lintOutput) } else { console.log(chalk.green('✔ No ESLint warnings or errors')) } diff --git a/packages/next/lib/eslint/customFormatter.ts b/packages/next/lib/eslint/customFormatter.ts index 5d956860a568f..9ed9abadc9e5b 100644 --- a/packages/next/lib/eslint/customFormatter.ts +++ b/packages/next/lib/eslint/customFormatter.ts @@ -15,7 +15,7 @@ interface LintMessage { column: number } -interface LintResult { +export interface LintResult { filePath: string messages: LintMessage[] errorCount: number @@ -28,7 +28,11 @@ function formatMessage( dir: string, messages: LintMessage[], filePath: string -): string | void { +): { + output: string + nextPluginErrorCount: number + nextPluginWarningCount: number +} { let fileName = path.posix.normalize( path.relative(dir, filePath).replace(/\\/g, '/') ) @@ -38,6 +42,8 @@ function formatMessage( } let output = '\n' + chalk.cyan(fileName) + let nextPluginWarningCount = 0 + let nextPluginErrorCount = 0 for (let i = 0; i < messages.length; i++) { const { message, severity, line, column, ruleId } = messages[i] @@ -53,6 +59,14 @@ function formatMessage( ' ' } + if (ruleId?.includes('@next/next')) { + if (severity === MessageSeverity.Warning) { + nextPluginWarningCount += 1 + } else { + nextPluginErrorCount += 1 + } + } + if (severity === MessageSeverity.Warning) { output += chalk.yellow.bold('Warning') + ': ' } else { @@ -66,19 +80,43 @@ function formatMessage( } } - return output + return { + output, + nextPluginErrorCount, + nextPluginWarningCount, + } } -export function formatResults(baseDir: string, results: LintResult[]): string { +export function formatResults( + baseDir: string, + results: LintResult[] +): { + output: string + totalNextPluginErrorCount: number + totalNextPluginWarningCount: number +} { + let totalNextPluginErrorCount = 0 + let totalNextPluginWarningCount = 0 + const formattedResults = results .filter(({ messages }) => messages?.length) - .map(({ messages, filePath }) => formatMessage(baseDir, messages, filePath)) + .map(({ messages, filePath }) => { + const res = formatMessage(baseDir, messages, filePath) + totalNextPluginErrorCount += res.nextPluginErrorCount + totalNextPluginWarningCount += res.nextPluginWarningCount + return res.output + }) .join('\n') - return formattedResults.length > 0 - ? formattedResults + - `\n\n${chalk.bold( - 'Need to disable some ESLint rules? Learn more here:' - )} https://nextjs.org/docs/basic-features/eslint#disabling-rules\n` - : '' + return { + output: + formattedResults.length > 0 + ? formattedResults + + `\n\n${chalk.bold( + 'Need to disable some ESLint rules? Learn more here:' + )} https://nextjs.org/docs/basic-features/eslint#disabling-rules\n` + : '', + totalNextPluginErrorCount, + totalNextPluginWarningCount, + } } diff --git a/packages/next/lib/eslint/runLintCheck.ts b/packages/next/lib/eslint/runLintCheck.ts index b176881efe220..d1e61a1424a3d 100644 --- a/packages/next/lib/eslint/runLintCheck.ts +++ b/packages/next/lib/eslint/runLintCheck.ts @@ -9,13 +9,13 @@ import * as CommentJson from 'next/dist/compiled/comment-json' import { formatResults } from './customFormatter' import { writeDefaultConfig } from './writeDefaultConfig' import { existsSync, findPagesDir } from '../find-pages-dir' -import { CompileError } from '../compile-error' import { hasNecessaryDependencies, NecessaryDependencies, } from '../has-necessary-dependencies' import * as Log from '../../build/output/log' +import { EventLintCheckCompleted } from '../../telemetry/events/build' type Config = { plugins: string[] @@ -29,14 +29,23 @@ async function lint( eslintrcFile: string | null, pkgJsonPath: string | null, eslintOptions: any = null -): Promise { +): Promise< + | string + | null + | { + output: string | null + isError: boolean + eventInfo: EventLintCheckCompleted + } +> { // Load ESLint after we're sure it exists: - const mod = await import(deps.resolved) + const mod = await import(deps.resolved.get('eslint')!) const { ESLint } = mod + let eslintVersion = ESLint.version if (!ESLint) { - const eslintVersion: string | undefined = mod?.CLIEngine?.version + eslintVersion = mod?.CLIEngine?.version if (!eslintVersion || semver.lt(eslintVersion, '7.0.0')) { return `${chalk.red( @@ -99,14 +108,33 @@ async function lint( eslint = new ESLint(options) } } + const lintStart = process.hrtime() + const results = await eslint.lintFiles(lintDirs) if (options.fix) await ESLint.outputFixes(results) - if (ESLint.getErrorResults(results)?.length > 0) { - throw new CompileError(await formatResults(baseDir, results)) + const formattedResult = formatResults(baseDir, results) + const lintEnd = process.hrtime(lintStart) + + return { + output: formattedResult.output, + isError: ESLint.getErrorResults(results)?.length > 0, + eventInfo: { + durationInSeconds: lintEnd[0], + eslintVersion: eslintVersion, + lintedFilesCount: results.length, + lintFix: !!options.fix, + nextEslintPluginVersion: nextEslintPluginIsEnabled + ? require(path.join( + path.dirname(deps.resolved.get('eslint-config-next')!), + 'package.json' + )).version + : null, + nextEslintPluginErrorsCount: formattedResult.totalNextPluginErrorCount, + nextEslintPluginWarningsCount: + formattedResult.totalNextPluginWarningCount, + }, } - - return results?.length > 0 ? formatResults(baseDir, results) : null } export async function runLintCheck( @@ -114,7 +142,7 @@ export async function runLintCheck( lintDirs: string[], lintDuringBuild: boolean = false, eslintOptions: any = null -): Promise { +): ReturnType { try { // Find user's .eslintrc file const eslintrcFile = diff --git a/packages/next/lib/has-necessary-dependencies.ts b/packages/next/lib/has-necessary-dependencies.ts index 22fb7e21c6c2d..ba62de0928232 100644 --- a/packages/next/lib/has-necessary-dependencies.ts +++ b/packages/next/lib/has-necessary-dependencies.ts @@ -17,7 +17,7 @@ const requiredLintPackages = [ ] export type NecessaryDependencies = { - resolved: string + resolved: Map } export async function hasNecessaryDependencies( @@ -46,9 +46,7 @@ export async function hasNecessaryDependencies( if (missingPackages.length < 1) { return { - resolved: checkESLintDeps - ? resolutions.get('eslint')! - : resolutions.get('typescript')!, + resolved: resolutions, } } diff --git a/packages/next/lib/verifyAndLint.ts b/packages/next/lib/verifyAndLint.ts index 3ee163b1b0f0b..0a36bbe78b1a6 100644 --- a/packages/next/lib/verifyAndLint.ts +++ b/packages/next/lib/verifyAndLint.ts @@ -3,12 +3,16 @@ import { Worker } from 'jest-worker' import { existsSync } from 'fs' import { join } from 'path' import { ESLINT_DEFAULT_DIRS } from './constants' +import { Telemetry } from '../telemetry/storage' +import { eventLintCheckCompleted } from '../telemetry/events' +import { CompileError } from './compile-error' export async function verifyAndLint( dir: string, configLintDirs: string[] | undefined, numWorkers: number | undefined, - enableWorkerThreads: boolean | undefined + enableWorkerThreads: boolean | undefined, + telemetry: Telemetry ): Promise { try { const lintWorkers = new Worker(require.resolve('./eslint/runLintCheck'), { @@ -32,13 +36,30 @@ export async function verifyAndLint( ) const lintResults = await lintWorkers.runLintCheck(dir, lintDirs, true) - if (lintResults) { - console.log(lintResults) + const lintOutput = + typeof lintResults === 'string' ? lintResults : lintResults?.output + + if (typeof lintResults !== 'string' && lintResults?.eventInfo) { + telemetry.record( + eventLintCheckCompleted({ + ...lintResults.eventInfo, + buildLint: true, + }) + ) + } + + if (typeof lintResults !== 'string' && lintResults?.isError && lintOutput) { + await telemetry.flush() + throw new CompileError(lintOutput) + } + + if (lintOutput) { + console.log(lintOutput) } lintWorkers.end() } catch (err) { - if (err.type === 'CompileError') { + if (err.type === 'CompileError' || err instanceof CompileError) { console.error(chalk.red('\nFailed to compile.')) console.error(err.message) process.exit(1) diff --git a/packages/next/lib/verifyTypeScriptSetup.ts b/packages/next/lib/verifyTypeScriptSetup.ts index 140c871240f23..8274e3d68195d 100644 --- a/packages/next/lib/verifyTypeScriptSetup.ts +++ b/packages/next/lib/verifyTypeScriptSetup.ts @@ -38,7 +38,9 @@ export async function verifyTypeScriptSetup( ) // Load TypeScript after we're sure it exists: - const ts = (await import(deps.resolved)) as typeof import('typescript') + const ts = (await import( + deps.resolved.get('typescript')! + )) as typeof import('typescript') if (semver.lt(ts.version, '4.3.2')) { log.warn( diff --git a/packages/next/telemetry/events/build.ts b/packages/next/telemetry/events/build.ts index 4ad75df441166..b47992afd96a0 100644 --- a/packages/next/telemetry/events/build.ts +++ b/packages/next/telemetry/events/build.ts @@ -20,6 +20,27 @@ export function eventTypeCheckCompleted( } } +const EVENT_LINT_CHECK_COMPLETED = 'NEXT_LINT_CHECK_COMPLETED' +export type EventLintCheckCompleted = { + durationInSeconds: number + eslintVersion: string | null + lintedFilesCount?: number + lintFix?: boolean + buildLint?: boolean + nextEslintPluginVersion?: string | null + nextEslintPluginErrorsCount?: number + nextEslintPluginWarningsCount?: number +} + +export function eventLintCheckCompleted( + event: EventLintCheckCompleted +): { eventName: string; payload: EventLintCheckCompleted } { + return { + eventName: EVENT_LINT_CHECK_COMPLETED, + payload: event, + } +} + const EVENT_BUILD_COMPLETED = 'NEXT_BUILD_COMPLETED' type EventBuildCompleted = { durationInSeconds: number diff --git a/test/integration/telemetry/test/index.test.js b/test/integration/telemetry/test/index.test.js index d5dd6589c982f..57f810ae081e4 100644 --- a/test/integration/telemetry/test/index.test.js +++ b/test/integration/telemetry/test/index.test.js @@ -9,6 +9,7 @@ import { killApp, waitFor, nextBuild, + nextLint, } from 'next-test-utils' jest.setTimeout(1000 * 60 * 2) @@ -517,4 +518,48 @@ describe('Telemetry CLI', () => { expect(event2).toMatch(/"trailingSlashEnabled": false/) expect(event2).toMatch(/"reactStrictMode": false/) }) + + it('emits telemetry for lint during build', async () => { + await fs.writeFile(path.join(appDir, '.eslintrc'), `{ "extends": "next" }`) + const { stderr } = await nextBuild(appDir, [], { + stderr: true, + env: { NEXT_TELEMETRY_DEBUG: 1 }, + }) + await fs.remove(path.join(appDir, '.eslintrc')) + + const event1 = /NEXT_LINT_CHECK_COMPLETED[\s\S]+?{([\s\S]+?)}/ + .exec(stderr) + .pop() + + expect(event1).toMatch(/"durationInSeconds": [\d]{1,}/) + expect(event1).toMatch(/"eslintVersion": ".*?\..*?\..*?"/) + expect(event1).toMatch(/"lintedFilesCount": [\d]{1,}/) + expect(event1).toMatch(/"lintFix": false/) + expect(event1).toMatch(/"buildLint": true/) + expect(event1).toMatch(/"nextEslintPluginVersion": ".*?\..*?\..*?"/) + expect(event1).toMatch(/"nextEslintPluginErrorsCount": \d{1,}/) + expect(event1).toMatch(/"nextEslintPluginWarningsCount": \d{1,}/) + }) + + it('emits telemetry for `next lint`', async () => { + await fs.writeFile(path.join(appDir, '.eslintrc'), `{ "extends": "next" }`) + const { stderr } = await nextLint(appDir, [], { + stderr: true, + env: { NEXT_TELEMETRY_DEBUG: 1 }, + }) + await fs.remove(path.join(appDir, '.eslintrc')) + + const event1 = /NEXT_LINT_CHECK_COMPLETED[\s\S]+?{([\s\S]+?)}/ + .exec(stderr) + .pop() + + expect(event1).toMatch(/"durationInSeconds": [\d]{1,}/) + expect(event1).toMatch(/"eslintVersion": ".*?\..*?\..*?"/) + expect(event1).toMatch(/"lintedFilesCount": [\d]{1,}/) + expect(event1).toMatch(/"lintFix": false/) + expect(event1).toMatch(/"buildLint": false/) + expect(event1).toMatch(/"nextEslintPluginVersion": ".*?\..*?\..*?"/) + expect(event1).toMatch(/"nextEslintPluginErrorsCount": \d{1,}/) + expect(event1).toMatch(/"nextEslintPluginWarningsCount": \d{1,}/) + }) })