Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect pnpm correctly when installing missing dependencies #37813

Merged
merged 3 commits into from
Jun 19, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions packages/next/lib/eslint/runLintCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand Down Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions packages/next/lib/helpers/get-pkg-manager.ts
Original file line number Diff line number Diff line change
@@ -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'
}
}
27 changes: 12 additions & 15 deletions packages/next/lib/helpers/install.ts
Original file line number Diff line number Diff line change
@@ -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.
*/
Expand All @@ -25,10 +26,10 @@ interface InstallArgs {
export function install(
root: string,
dependencies: string[] | null,
{ useYarn, isOnline, devDependencies }: InstallArgs
{ packageManager, isOnline, devDependencies }: InstallArgs
): Promise<void> {
/**
* NPM-specific command-line flags.
* (p)npm-specific command-line flags.
*/
const npmFlags: string[] = []
/**
Expand All @@ -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) {
/**
Expand All @@ -57,15 +59,15 @@ 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')
args.push(...dependencies)
}
} else {
/**
* If there are no dependencies, run a variation of `{displayCommand}
* If there are no dependencies, run a variation of `{packageManager}
* install`.
*/
args = ['install']
Expand Down Expand Up @@ -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) {
Expand Down
22 changes: 0 additions & 22 deletions packages/next/lib/helpers/should-use-yarn.ts

This file was deleted.

15 changes: 9 additions & 6 deletions packages/next/lib/install-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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()
}
Expand Down
5 changes: 0 additions & 5 deletions packages/next/lib/is-yarn.ts

This file was deleted.

9 changes: 7 additions & 2 deletions packages/next/lib/typescript/missingDependencyError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ 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,
missingPackages: MissingDependency[]
) {
const packagesHuman = getOxfordCommaList(missingPackages.map((p) => p.pkg))
const packagesCli = missingPackages.map((p) => p.pkg).join(' ')
const packageManager = getPkgManager(dir)

const removalMsg =
'\n\n' +
Expand All @@ -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
)}` +
Expand Down
12 changes: 8 additions & 4 deletions packages/next/lib/verify-partytown-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand All @@ -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(
Expand Down
46 changes: 28 additions & 18 deletions test/integration/eslint/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions test/integration/script-loader/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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|add) (--save-dev|--dev) @builder.io\/partytownIf you are not trying to use Partytown, please disable the experimental "nextScriptWorkers" flag in next.config.js./
)
})
})