From c21bf9cbc02ba12821f76527dccd1c6fabd34d04 Mon Sep 17 00:00:00 2001 From: Willem-Jaap <67187467+Willem-Jaap@users.noreply.github.com> Date: Tue, 23 Jul 2024 21:49:18 +0200 Subject: [PATCH] Update Next Command name, add types & update variable wording (#68007) ### What? This PR adds updates the command name and adds argument types to the `next` script to improve internal DX, code quality and correctness. --- packages/next/src/bin/next.ts | 81 ++++++++++++++----------- packages/next/src/cli/next-build.ts | 3 +- packages/next/src/cli/next-dev.ts | 2 +- packages/next/src/cli/next-info.ts | 2 +- packages/next/src/cli/next-lint.ts | 2 +- packages/next/src/cli/next-start.ts | 14 +++-- packages/next/src/cli/next-telemetry.ts | 2 +- packages/next/src/lib/is-error.ts | 6 +- packages/next/src/server/lib/utils.ts | 8 ++- 9 files changed, 73 insertions(+), 47 deletions(-) diff --git a/packages/next/src/bin/next.ts b/packages/next/src/bin/next.ts index 9e278db7d1838..0711197a56b5c 100755 --- a/packages/next/src/bin/next.ts +++ b/packages/next/src/bin/next.ts @@ -9,8 +9,17 @@ import semver from 'next/dist/compiled/semver' import { bold, cyan, italic } from '../lib/picocolors' import { formatCliHelpOutput } from '../lib/format-cli-help-output' import { NON_STANDARD_NODE_ENV } from '../lib/constants' -import { myParseInt } from '../server/lib/utils' -import { SUPPORTED_TEST_RUNNERS_LIST } from '../cli/next-test.js' +import { parseValidPositiveInteger } from '../server/lib/utils' +import { + SUPPORTED_TEST_RUNNERS_LIST, + type NextTestOptions, +} from '../cli/next-test.js' +import type { NextTelemetryOptions } from '../cli/next-telemetry.js' +import type { NextStartOptions } from '../cli/next-start.js' +import type { NextLintOptions } from '../cli/next-lint.js' +import type { NextInfoOptions } from '../cli/next-info.js' +import type { NextDevOptions } from '../cli/next-dev.js' +import type { NextBuildOptions } from '../cli/next-build.js' if ( semver.lt(process.versions.node, process.env.__NEXT_REQUIRED_NODE_VERSION!) @@ -35,15 +44,15 @@ for (const dependency of ['react', 'react-dom']) { } } -class MyRootCommand extends Command { +class NextRootCommand extends Command { createCommand(name: string) { - const cmd = new Command(name) + const command = new Command(name) - cmd.addOption(new Option('--inspect').hideHelp()) + command.addOption(new Option('--inspect').hideHelp()) - cmd.hook('preAction', (thisCommand) => { - const cmdName = thisCommand.name() - const defaultEnv = cmdName === 'dev' ? 'development' : 'production' + command.hook('preAction', (event) => { + const commandName = event.name() + const defaultEnv = commandName === 'dev' ? 'development' : 'production' const standardEnv = ['production', 'development', 'test'] if (process.env.NODE_ENV) { @@ -55,7 +64,7 @@ class MyRootCommand extends Command { ? ['dev'] : [] - if (isNotStandard || shouldWarnCommands.includes(cmdName)) { + if (isNotStandard || shouldWarnCommands.includes(commandName)) { warn(NON_STANDARD_NODE_ENV) } } @@ -63,19 +72,19 @@ class MyRootCommand extends Command { ;(process.env as any).NODE_ENV = process.env.NODE_ENV || defaultEnv ;(process.env as any).NEXT_RUNTIME = 'nodejs' - if (thisCommand.getOptionValue('inspect') === true) { + if (event.getOptionValue('inspect') === true) { console.error( - `\`--inspect\` flag is deprecated. Use env variable NODE_OPTIONS instead: NODE_OPTIONS='--inspect' next ${cmdName}` + `\`--inspect\` flag is deprecated. Use env variable NODE_OPTIONS instead: NODE_OPTIONS='--inspect' next ${commandName}` ) process.exit(1) } }) - return cmd + return command } } -const program = new MyRootCommand() +const program = new NextRootCommand() program .name('next') @@ -128,7 +137,7 @@ program '--experimental-upload-trace, ', 'Reports a subset of the debugging trace to a remote HTTP URL. Includes sensitive data.' ) - .action((directory, options) => + .action((directory: string, options: NextBuildOptions) => // ensure process exits after build completes so open handles/connections // don't cause process to hang import('../cli/next-build.js').then((mod) => @@ -154,7 +163,7 @@ program '-p, --port ', 'Specify a port number on which to start the application.' ) - .argParser(myParseInt) + .argParser(parseValidPositiveInteger) .default(3000) .env('PORT') ) @@ -179,12 +188,14 @@ program '--experimental-upload-trace, ', 'Reports a subset of the debugging trace to a remote HTTP URL. Includes sensitive data.' ) - .action((directory, options, { _optionValueSources }) => { - const portSource = _optionValueSources.port - import('../cli/next-dev.js').then((mod) => - mod.nextDev(options, portSource, directory) - ) - }) + .action( + (directory: string, options: NextDevOptions, { _optionValueSources }) => { + const portSource = _optionValueSources.port + import('../cli/next-dev.js').then((mod) => + mod.nextDev(options, portSource, directory) + ) + } + ) .usage('[directory] [options]') program @@ -202,7 +213,7 @@ program `\nLearn more: ${cyan('https://nextjs.org/docs/api-reference/cli#info')}` ) .option('--verbose', 'Collects additional information for debugging.') - .action((options) => + .action((options: NextInfoOptions) => import('../cli/next-info.js').then((mod) => mod.nextInfo(options)) ) @@ -257,7 +268,7 @@ program '--max-warnings [maxWarnings]', 'Specify the number of warnings before triggering a non-zero exit code.' ) - .argParser(myParseInt) + .argParser(parseValidPositiveInteger) .default(-1) ) .option( @@ -287,7 +298,7 @@ program '--error-on-unmatched-pattern', 'Reports errors when any file patterns are unmatched.' ) - .action((directory, options) => + .action((directory: string, options: NextLintOptions) => import('../cli/next-lint.js').then((mod) => mod.nextLint(options, directory) ) @@ -310,7 +321,7 @@ program '-p, --port ', 'Specify a port number on which to start the application.' ) - .argParser(myParseInt) + .argParser(parseValidPositiveInteger) .default(3000) .env('PORT') ) @@ -322,9 +333,9 @@ program new Option( '--keepAliveTimeout ', 'Specify the maximum amount of milliseconds to wait before closing inactive connections.' - ).argParser(myParseInt) + ).argParser(parseValidPositiveInteger) ) - .action((directory, options) => + .action((directory: string, options: NextStartOptions) => import('../cli/next-start.js').then((mod) => mod.nextStart(options, directory) ) @@ -346,7 +357,7 @@ program ) ) .option('--disable', `Disables Next.js' telemetry collection.`) - .action((arg, options) => + .action((arg: string, options: NextTelemetryOptions) => import('../cli/next-telemetry.js').then((mod) => mod.nextTelemetry(options, arg) ) @@ -376,11 +387,13 @@ program )}` ) .allowUnknownOption() - .action((directory, testRunnerArgs, options) => { - return import('../cli/next-test.js').then((mod) => { - mod.nextTest(directory, testRunnerArgs, options) - }) - }) + .action( + (directory: string, testRunnerArgs: string[], options: NextTestOptions) => { + return import('../cli/next-test.js').then((mod) => { + mod.nextTest(directory, testRunnerArgs, options) + }) + } + ) .usage('[directory] [options]') const internal = program @@ -392,7 +405,7 @@ const internal = program internal .command('turbo-trace-server') .argument('[file]', 'Trace file to serve.') - .action((file) => { + .action((file: string) => { return import('../cli/internal/turbo-trace-server.js').then((mod) => mod.startTurboTraceServerCli(file) ) diff --git a/packages/next/src/cli/next-build.ts b/packages/next/src/cli/next-build.ts index dbbdff5b8c5e1..aee2877aafd6e 100755 --- a/packages/next/src/cli/next-build.ts +++ b/packages/next/src/cli/next-build.ts @@ -11,7 +11,7 @@ import { getProjectDir } from '../lib/get-project-dir' import { enableMemoryDebuggingMode } from '../lib/memory/startup' import { disableMemoryDebuggingMode } from '../lib/memory/shutdown' -type NextBuildOptions = { +export type NextBuildOptions = { debug?: boolean profile?: boolean lint: boolean @@ -67,7 +67,6 @@ const nextBuild = (options: NextBuildOptions, directory?: string) => { const dir = getProjectDir(directory) - // Check if the provided directory exists if (!existsSync(dir)) { printAndExit(`> No such directory exists as the project root: ${dir}`) } diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index c53056109f0f6..06c1bff533170 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -36,7 +36,7 @@ import { import os from 'os' import { once } from 'node:events' -type NextDevOptions = { +export type NextDevOptions = { turbo?: boolean port: number hostname?: string diff --git a/packages/next/src/cli/next-info.ts b/packages/next/src/cli/next-info.ts index cb7be5b656a5b..bbcd59c5f9464 100755 --- a/packages/next/src/cli/next-info.ts +++ b/packages/next/src/cli/next-info.ts @@ -11,7 +11,7 @@ import { parseVersionInfo } from '../server/dev/parse-version-info' import { getStaleness } from '../client/components/react-dev-overlay/internal/components/VersionStalenessInfo/VersionStalenessInfo' import { warn } from '../build/output/log' -type NextInfoOptions = { +export type NextInfoOptions = { verbose?: boolean } diff --git a/packages/next/src/cli/next-lint.ts b/packages/next/src/cli/next-lint.ts index 9fc2090eac530..644fac6fd1c71 100755 --- a/packages/next/src/cli/next-lint.ts +++ b/packages/next/src/cli/next-lint.ts @@ -16,7 +16,7 @@ import { getProjectDir } from '../lib/get-project-dir' import { findPagesDir } from '../lib/find-pages-dir' import { verifyTypeScriptSetup } from '../lib/verify-typescript-setup' -type NextLintOptions = { +export type NextLintOptions = { cache: boolean cacheLocation?: string cacheStrategy: string diff --git a/packages/next/src/cli/next-start.ts b/packages/next/src/cli/next-start.ts index 90992d54d602b..8be522893356c 100755 --- a/packages/next/src/cli/next-start.ts +++ b/packages/next/src/cli/next-start.ts @@ -9,17 +9,23 @@ import { isPortIsReserved, } from '../lib/helpers/get-reserved-port' -type NextStartOptions = { +export type NextStartOptions = { port: number hostname?: string keepAliveTimeout?: number } +/** + * Start the Next.js server + * + * @param options The options for the start command + * @param directory The directory to start the server in + */ const nextStart = async (options: NextStartOptions, directory?: string) => { const dir = getProjectDir(directory) - const host = options.hostname + const hostname = options.hostname const port = options.port - let keepAliveTimeout = options.keepAliveTimeout + const keepAliveTimeout = options.keepAliveTimeout if (isPortIsReserved(port)) { printAndExit(getReservedPortExplanation(port), 1) @@ -28,7 +34,7 @@ const nextStart = async (options: NextStartOptions, directory?: string) => { await startServer({ dir, isDev: false, - hostname: host, + hostname, port, keepAliveTimeout, }) diff --git a/packages/next/src/cli/next-telemetry.ts b/packages/next/src/cli/next-telemetry.ts index deea300d5bafb..cf922f68360f7 100755 --- a/packages/next/src/cli/next-telemetry.ts +++ b/packages/next/src/cli/next-telemetry.ts @@ -3,7 +3,7 @@ import { bold, cyan, green, red, yellow } from '../lib/picocolors' import { Telemetry } from '../telemetry/storage' -type NextTelemetryOptions = { +export type NextTelemetryOptions = { enable?: boolean disable?: boolean } diff --git a/packages/next/src/lib/is-error.ts b/packages/next/src/lib/is-error.ts index f9bf9104a3d2c..e8147912ae162 100644 --- a/packages/next/src/lib/is-error.ts +++ b/packages/next/src/lib/is-error.ts @@ -1,6 +1,6 @@ import { isPlainObject } from '../shared/lib/is-plain-object' -// We allow some additional attached properties for Errors +// We allow some additional attached properties for Next.js errors export interface NextError extends Error { type?: string page?: string @@ -9,6 +9,10 @@ export interface NextError extends Error { digest?: number } +/** + * Checks whether the given value is a NextError. + * This can be used to print a more detailed error message with properties like `code` & `digest`. + */ export default function isError(err: unknown): err is NextError { return ( typeof err === 'object' && err !== null && 'name' in err && 'message' in err diff --git a/packages/next/src/server/lib/utils.ts b/packages/next/src/server/lib/utils.ts index 2c6a213f9fc09..4b16482fe5a59 100644 --- a/packages/next/src/server/lib/utils.ts +++ b/packages/next/src/server/lib/utils.ts @@ -245,8 +245,12 @@ export function getFormattedNodeOptionsWithoutInspect() { return formatNodeOptions(args) } -export function myParseInt(value: string) { - // parseInt takes a string and a radix +/** + * Check if the value is a valid positive integer and parse it. If it's not, it will throw an error. + * + * @param value The value to be parsed. + */ +export function parseValidPositiveInteger(value: string): number { const parsedValue = parseInt(value, 10) if (isNaN(parsedValue) || !isFinite(parsedValue) || parsedValue < 0) {