From 45dee773eac3c8ddcc8527e09faf038020aac0f8 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sat, 18 Jun 2022 18:25:08 -0500 Subject: [PATCH 1/2] Detect pnpm correctly when installing missing dependencies --- packages/next/lib/eslint/runLintCheck.ts | 11 +++-- packages/next/lib/helpers/get-pkg-manager.ts | 36 +++++++++++++++ packages/next/lib/helpers/install.ts | 27 +++++------ packages/next/lib/helpers/should-use-yarn.ts | 22 --------- packages/next/lib/install-dependencies.ts | 15 +++--- packages/next/lib/is-yarn.ts | 5 -- .../lib/typescript/missingDependencyError.ts | 9 +++- packages/next/lib/verify-partytown-setup.ts | 12 +++-- test/integration/eslint/test/index.test.js | 46 +++++++++++-------- .../script-loader/test/index.test.js | 4 +- 10 files changed, 109 insertions(+), 78 deletions(-) create mode 100644 packages/next/lib/helpers/get-pkg-manager.ts delete mode 100644 packages/next/lib/helpers/should-use-yarn.ts delete mode 100644 packages/next/lib/is-yarn.ts diff --git a/packages/next/lib/eslint/runLintCheck.ts b/packages/next/lib/eslint/runLintCheck.ts index e994f8528680c..de9fc1c7a4deb 100644 --- a/packages/next/lib/eslint/runLintCheck.ts +++ b/packages/next/lib/eslint/runLintCheck.ts @@ -14,11 +14,11 @@ import { ESLINT_PROMPT_VALUES } from '../constants' import { existsSync, findPagesDir } from '../find-pages-dir' import { installDependencies } from '../install-dependencies' import { hasNecessaryDependencies } from '../has-necessary-dependencies' -import { isYarn } from '../is-yarn' import * as Log from '../../build/output/log' import { EventLintCheckCompleted } from '../../telemetry/events/build' import isError, { getProperError } from '../is-error' +import { getPkgManager } from '../helpers/get-pkg-manager' type Config = { plugins: string[] @@ -99,15 +99,18 @@ async function lint( try { // Load ESLint after we're sure it exists: const deps = await hasNecessaryDependencies(baseDir, requiredPackages) + const packageManager = getPkgManager(baseDir) if (deps.missing.some((dep) => dep.pkg === 'eslint')) { Log.error( `ESLint must be installed${ lintDuringBuild ? ' in order to run during builds:' : ':' } ${chalk.bold.cyan( - (await isYarn(baseDir)) - ? 'yarn add --dev eslint' - : 'npm install --save-dev eslint' + (packageManager === 'yarn' + ? 'yarn add --dev' + : packageManager === 'pnpm' + ? 'pnpm install --save-dev' + : 'npm install --save-dev') + ' eslint' )}` ) return null diff --git a/packages/next/lib/helpers/get-pkg-manager.ts b/packages/next/lib/helpers/get-pkg-manager.ts new file mode 100644 index 0000000000000..7d05594b7e636 --- /dev/null +++ b/packages/next/lib/helpers/get-pkg-manager.ts @@ -0,0 +1,36 @@ +import fs from 'fs' +import path from 'path' +import { execSync } from 'child_process' + +export type PackageManager = 'npm' | 'pnpm' | 'yarn' + +export function getPkgManager(baseDir: string): PackageManager { + try { + for (const { lockFile, packageManager } of [ + { lockFile: 'yarn.lock', packageManager: 'yarn' }, + { lockFile: 'pnpm-lock.yaml', packageManager: 'pnpm' }, + { lockFile: 'package-lock.json', packageManager: 'npm' }, + ]) { + if (fs.existsSync(path.join(baseDir, lockFile))) { + return packageManager as PackageManager + } + } + const userAgent = process.env.npm_config_user_agent + if (userAgent) { + if (userAgent.startsWith('yarn')) { + return 'yarn' + } else if (userAgent.startsWith('pnpm')) { + return 'pnpm' + } + } + try { + execSync('yarn --version', { stdio: 'ignore' }) + return 'yarn' + } catch { + execSync('pnpm --version', { stdio: 'ignore' }) + return 'pnpm' + } + } catch { + return 'npm' + } +} diff --git a/packages/next/lib/helpers/install.ts b/packages/next/lib/helpers/install.ts index 1d082e29b5803..f6d252a0b43ff 100644 --- a/packages/next/lib/helpers/install.ts +++ b/packages/next/lib/helpers/install.ts @@ -1,12 +1,13 @@ -/* eslint-disable import/no-extraneous-dependencies */ import chalk from 'next/dist/compiled/chalk' import spawn from 'next/dist/compiled/cross-spawn' +export type PackageManager = 'npm' | 'pnpm' | 'yarn' + interface InstallArgs { /** - * Indicate whether to install packages using Yarn. + * Indicate whether to install packages using npm, pnpm or Yarn. */ - useYarn: boolean + packageManager: PackageManager /** * Indicate whether there is an active Internet connection. */ @@ -25,10 +26,10 @@ interface InstallArgs { export function install( root: string, dependencies: string[] | null, - { useYarn, isOnline, devDependencies }: InstallArgs + { packageManager, isOnline, devDependencies }: InstallArgs ): Promise { /** - * NPM-specific command-line flags. + * (p)npm-specific command-line flags. */ const npmFlags: string[] = [] /** @@ -40,11 +41,12 @@ export function install( */ return new Promise((resolve, reject) => { let args: string[] - let command: string = useYarn ? 'yarnpkg' : 'npm' + let command = packageManager + const useYarn = packageManager === 'yarn' if (dependencies && dependencies.length) { /** - * If there are dependencies, run a variation of `{displayCommand} add`. + * If there are dependencies, run a variation of `{packageManager} add`. */ if (useYarn) { /** @@ -57,7 +59,7 @@ export function install( args.push(...dependencies) } else { /** - * Call `npm install [--save|--save-dev] ...`. + * Call `(p)npm install [--save|--save-dev] ...`. */ args = ['install', '--save-exact'] args.push(devDependencies ? '--save-dev' : '--save') @@ -65,7 +67,7 @@ export function install( } } else { /** - * If there are no dependencies, run a variation of `{displayCommand} + * If there are no dependencies, run a variation of `{packageManager} * install`. */ args = ['install'] @@ -93,12 +95,7 @@ export function install( */ const child = spawn(command, args, { stdio: 'inherit', - env: { - ...process.env, - NODE_ENV: devDependencies ? 'development' : 'production', - ADBLOCK: '1', - DISABLE_OPENCOLLECTIVE: '1', - }, + env: { ...process.env, ADBLOCK: '1', DISABLE_OPENCOLLECTIVE: '1' }, }) child.on('close', (code) => { if (code !== 0) { diff --git a/packages/next/lib/helpers/should-use-yarn.ts b/packages/next/lib/helpers/should-use-yarn.ts deleted file mode 100644 index 27c15b1dbac7d..0000000000000 --- a/packages/next/lib/helpers/should-use-yarn.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { execSync } from 'child_process' -import fs from 'fs' -import path from 'path' - -export function shouldUseYarn(baseDir: string): boolean { - try { - const userAgent = process.env.npm_config_user_agent - if (userAgent) { - return Boolean(userAgent && userAgent.startsWith('yarn')) - } else { - if (fs.existsSync(path.join(baseDir, 'yarn.lock'))) { - return true - } else if (fs.existsSync(path.join(baseDir, 'package-lock.json'))) { - return false - } - execSync('yarnpkg --version', { stdio: 'ignore' }) - return true - } - } catch (e) { - return false - } -} diff --git a/packages/next/lib/install-dependencies.ts b/packages/next/lib/install-dependencies.ts index 77987f0ddf7bd..ce23435113ce8 100644 --- a/packages/next/lib/install-dependencies.ts +++ b/packages/next/lib/install-dependencies.ts @@ -2,7 +2,7 @@ import chalk from 'next/dist/compiled/chalk' import path from 'path' import { MissingDependency } from './has-necessary-dependencies' -import { shouldUseYarn } from './helpers/should-use-yarn' +import { getPkgManager } from './helpers/get-pkg-manager' import { install } from './helpers/install' import { getOnline } from './helpers/get-online' @@ -15,22 +15,25 @@ export async function installDependencies( deps: any, dev: boolean = false ) { - const useYarn = shouldUseYarn(baseDir) - const isOnline = !useYarn || (await getOnline()) + const packageManager = getPkgManager(baseDir) + const isOnline = await getOnline() if (deps.length) { console.log() - console.log(`Installing ${dev ? 'devDependencies' : 'dependencies'}:`) + console.log( + `Installing ${ + dev ? 'devDependencies' : 'dependencies' + } (${packageManager}):` + ) for (const dep of deps) { console.log(`- ${chalk.cyan(dep.pkg)}`) } console.log() - const devInstallFlags = { devDependencies: dev, ...{ useYarn, isOnline } } await install( path.resolve(baseDir), deps.map((dep: MissingDependency) => dep.pkg), - devInstallFlags + { devDependencies: dev, isOnline, packageManager } ) console.log() } diff --git a/packages/next/lib/is-yarn.ts b/packages/next/lib/is-yarn.ts deleted file mode 100644 index cbee420209717..0000000000000 --- a/packages/next/lib/is-yarn.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { join } from 'path' -import { fileExists } from './file-exists' - -export const isYarn = async (dir: string) => - await fileExists(join(dir, 'yarn.lock')).catch(() => false) diff --git a/packages/next/lib/typescript/missingDependencyError.ts b/packages/next/lib/typescript/missingDependencyError.ts index 44e7e2da8d8c8..49c21d99d07a0 100644 --- a/packages/next/lib/typescript/missingDependencyError.ts +++ b/packages/next/lib/typescript/missingDependencyError.ts @@ -3,7 +3,7 @@ import chalk from 'next/dist/compiled/chalk' import { getOxfordCommaList } from '../oxford-comma-list' import { MissingDependency } from '../has-necessary-dependencies' import { FatalError } from '../fatal-error' -import { isYarn } from '../is-yarn' +import { getPkgManager } from '../helpers/get-pkg-manager' export async function missingDepsError( dir: string, @@ -11,6 +11,7 @@ export async function missingDepsError( ) { const packagesHuman = getOxfordCommaList(missingPackages.map((p) => p.pkg)) const packagesCli = missingPackages.map((p) => p.pkg).join(' ') + const packageManager = getPkgManager(dir) const removalMsg = '\n\n' + @@ -28,7 +29,11 @@ export async function missingDepsError( chalk.bold(`Please install ${chalk.bold(packagesHuman)} by running:`) + '\n\n' + `\t${chalk.bold.cyan( - ((await isYarn(dir)) ? 'yarn add --dev' : 'npm install --save-dev') + + (packageManager === 'yarn' + ? 'yarn add --dev' + : packageManager === 'pnpm' + ? 'pnpm install --save-dev' + : 'npm install --save-dev') + ' ' + packagesCli )}` + diff --git a/packages/next/lib/verify-partytown-setup.ts b/packages/next/lib/verify-partytown-setup.ts index cabae4723d9bf..e98848eba8c34 100644 --- a/packages/next/lib/verify-partytown-setup.ts +++ b/packages/next/lib/verify-partytown-setup.ts @@ -6,13 +6,15 @@ import { hasNecessaryDependencies, NecessaryDependencies, } from './has-necessary-dependencies' -import { isYarn } from './is-yarn' import { fileExists } from './file-exists' import { FatalError } from './fatal-error' import { recursiveDelete } from './recursive-delete' import * as Log from '../build/output/log' +import { getPkgManager } from './helpers/get-pkg-manager' async function missingDependencyError(dir: string) { + const packageManager = getPkgManager(dir) + throw new FatalError( chalk.bold.red( "It looks like you're trying to use Partytown with next/script but do not have the required package(s) installed." @@ -21,9 +23,11 @@ async function missingDependencyError(dir: string) { chalk.bold(`Please install Partytown by running:`) + '\n\n' + `\t${chalk.bold.cyan( - (await isYarn(dir)) - ? 'yarn add @builder.io/partytown' - : 'npm install @builder.io/partytown' + (packageManager === 'yarn' + ? 'yarn add --dev' + : packageManager === 'pnpm' + ? 'pnpm install --save-dev' + : 'npm install --save-dev') + ' @builder.io/partytown' )}` + '\n\n' + chalk.bold( diff --git a/test/integration/eslint/test/index.test.js b/test/integration/eslint/test/index.test.js index 9a3f23bf25d76..f6c252dc47a32 100644 --- a/test/integration/eslint/test/index.test.js +++ b/test/integration/eslint/test/index.test.js @@ -185,24 +185,23 @@ describe('ESLint', () => { describe('Next Lint', () => { describe('First Time Setup ', () => { - async function nextLintTemp() { + async function nextLintTemp(setupCallback) { const folder = join( os.tmpdir(), Math.random().toString(36).substring(2) ) await fs.mkdirp(folder) await fs.copy(dirNoConfig, folder) + await setupCallback?.(folder) try { - const nextDir = dirname(require.resolve('next/package')) - const nextBin = join(nextDir, 'dist/bin/next') + const { stdout, stderr } = await nextLint(folder, ['--strict'], { + stderr: true, + stdout: true, + cwd: folder, + }) - const { stdout } = await execa('node', [ - nextBin, - 'lint', - folder, - '--strict', - ]) + console.log({ stdout, stderr }) const pkgJson = JSON.parse( await fs.readFile(join(folder, 'package.json'), 'utf8') @@ -234,15 +233,26 @@ describe('ESLint', () => { expect(output).toContain('Cancel') }) - test('installs eslint and eslint-config-next as devDependencies if missing', async () => { - const { stdout, pkgJson } = await nextLintTemp() - - expect(stdout.replace(/(\r\n|\n|\r)/gm, '')).toContain( - 'Installing devDependencies:- eslint- eslint-config-next' - ) - expect(pkgJson.devDependencies).toHaveProperty('eslint') - expect(pkgJson.devDependencies).toHaveProperty('eslint-config-next') - }) + for (const { packageManger, lockFile } of [ + { packageManger: 'yarn', lockFile: 'yarn.lock' }, + { packageManger: 'pnpm', lockFile: 'pnpm-lock.yaml' }, + { packageManger: 'npm', lockFile: 'package-lock.json' }, + ]) { + test(`installs eslint and eslint-config-next as devDependencies if missing with ${packageManger}`, async () => { + const { stdout, pkgJson } = await nextLintTemp(async (folder) => { + await fs.writeFile(join(folder, lockFile), '') + }) + + expect(stdout).toContain( + `Installing devDependencies (${packageManger}):` + ) + expect(stdout).toContain('eslint') + expect(stdout).toContain('eslint-config-next') + expect(stdout).toContain(packageManger) + expect(pkgJson.devDependencies).toHaveProperty('eslint') + expect(pkgJson.devDependencies).toHaveProperty('eslint-config-next') + }) + } test('creates .eslintrc.json file with a default configuration', async () => { const { stdout, eslintrcJson } = await nextLintTemp() diff --git a/test/integration/script-loader/test/index.test.js b/test/integration/script-loader/test/index.test.js index 89d3f4008c0f8..a8577fd1413e7 100644 --- a/test/integration/script-loader/test/index.test.js +++ b/test/integration/script-loader/test/index.test.js @@ -252,8 +252,8 @@ describe('Next.js Script - Primary Strategies', () => { }) const output = stdout + stderr - expect(output.replace(/\n|\r/g, '')).toContain( - `It looks like you're trying to use Partytown with next/script but do not have the required package(s) installed.Please install Partytown by running: npm install @builder.io/partytownIf you are not trying to use Partytown, please disable the experimental "nextScriptWorkers" flag in next.config.js.` + expect(output.replace(/\n|\r/g, '')).toMatch( + /It looks like you're trying to use Partytown with next\/script but do not have the required package\(s\) installed.Please install Partytown by running:.*?(npm|pnpm|yarn) install (--save-dev|--dev) @builder.io\/partytownIf you are not trying to use Partytown, please disable the experimental "nextScriptWorkers" flag in next.config.js./ ) }) }) From f13d5395f05b080dedb11410b314f63d91fd74a5 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sat, 18 Jun 2022 18:41:01 -0500 Subject: [PATCH 2/2] update test --- test/integration/script-loader/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/script-loader/test/index.test.js b/test/integration/script-loader/test/index.test.js index a8577fd1413e7..64cbd76f0981b 100644 --- a/test/integration/script-loader/test/index.test.js +++ b/test/integration/script-loader/test/index.test.js @@ -253,7 +253,7 @@ describe('Next.js Script - Primary Strategies', () => { const output = stdout + stderr expect(output.replace(/\n|\r/g, '')).toMatch( - /It looks like you're trying to use Partytown with next\/script but do not have the required package\(s\) installed.Please install Partytown by running:.*?(npm|pnpm|yarn) install (--save-dev|--dev) @builder.io\/partytownIf you are not trying to use Partytown, please disable the experimental "nextScriptWorkers" flag in next.config.js./ + /It looks like you're trying to use Partytown with next\/script but do not have the required package\(s\) installed.Please install Partytown by running:.*?(npm|pnpm|yarn) (install|add) (--save-dev|--dev) @builder.io\/partytownIf you are not trying to use Partytown, please disable the experimental "nextScriptWorkers" flag in next.config.js./ ) }) })