Skip to content

Commit

Permalink
fix: totally rework package installation to peer deps
Browse files Browse the repository at this point in the history
  • Loading branch information
nlf committed Sep 1, 2021
1 parent d955ef3 commit d1380df
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 113 deletions.
29 changes: 29 additions & 0 deletions bin/npm-template-check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env node

const check = require('../lib/check.js')

const main = async () => {
const {
npm_config_local_prefix: root,
} = process.env

if (!root) {
throw new Error('This package requires npm >7.21.1')
}

const problems = await check(root)
if (problems.length) {
console.error('Some problems were detected:')
console.error()
console.error(problems.map((problem) => problem.message).join('\n'))
console.error()
console.error('To correct them:')
console.error(problems.map((problem) => problem.solution).join('\n'))
process.exitCode = 1
}
}

module.exports = main().catch((err) => {
console.error(err.stack)
process.exitCode = 1
})
4 changes: 1 addition & 3 deletions bin/postinstall.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#!/usr/bin/env node

const copyContent = require('../lib/content/index.js')
const installPackages = require('../lib/install.js')
const patchPackage = require('../lib/package.js')

const main = async () => {
Expand All @@ -20,8 +19,7 @@ const main = async () => {
return
}

await copyContent(root)
return installPackages(root)
return copyContent(root)
}

// we export the promise so it can be awaited in tests, coverage is disabled
Expand Down
95 changes: 95 additions & 0 deletions lib/check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
const { join } = require('path')
const fs = require('@npmcli/fs')

const patchPackage = require('./package.js')

const unwantedPackages = [
'@npmcli/lint',
'eslint-plugin-promise',
'eslint-plugin-standard',
'eslint-plugin-import',
]

const hasOwn = (obj, key) => Object.prototype.hasOwnProperty.call(obj, key)

const check = async (root) => {
const pkgPath = join(root, 'package.json')
try {
var contents = await fs.readFile(pkgPath, { encoding: 'utf8' })
} catch (err) {
throw new Error('No package.json found')
}

const pkg = JSON.parse(contents)

const problems = []

const incorrectFields = []
// 1. ensure package.json changes have been applied
for (const [key, value] of Object.entries(patchPackage.changes)) {
if (!hasOwn(pkg, key)) {
incorrectFields.push({
name: key,
found: pkg[key],
expected: value,
})
continue
}

if (value && typeof value === 'object') {
for (const [subKey, subValue] of Object.entries(value)) {
if (!hasOwn(pkg[key], subKey) ||
pkg[key][subKey] !== subValue) {
incorrectFields.push({
name: `${key}.${subKey}`,
found: pkg[key][subKey],
expected: subValue,
})
}
}
} else {
if (pkg[key] !== patchPackage.changes[key]) {
incorrectFields.push({
name: key,
found: pkg[key],
expected: value,
})
}
}
}

if (incorrectFields.length) {
problems.push({
message: [
`The following package.json fields are incorrect:`,
...incorrectFields.map((field) => {
const { name, found, expected } = field
return ` Field: "${name}" Expected: "${expected}" Found: "${found}"`
}),
].join('\n'),
solution: 'npm rm @npmcli/template-oss && npm i -D @npmcli/template-oss',
})
}

// 2. ensure packages that should not be present are removed
const mustRemove = unwantedPackages.filter((name) => {
return hasOwn(pkg.dependencies || {}, name) ||
hasOwn(pkg.devDependencies || {}, name)
})

if (mustRemove.length) {
problems.push({
message: [
'The following unwanted packages were found:',
...mustRemove,
].join(' '),
solution: `npm rm ${mustRemove.join(' ')}`,
})
}

return problems
}

check.unwantedPackages = unwantedPackages

module.exports = check
39 changes: 0 additions & 39 deletions lib/install.js

This file was deleted.

1 change: 1 addition & 0 deletions lib/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const changes = {
templateVersion: TEMPLATE_VERSION,
scripts: {
lint: `eslint '**/*.js'`,
postlint: 'npm-template-check',
lintfix: 'npm run lint -- --fix',
preversion: 'npm test',
postversion: 'npm publish',
Expand Down
25 changes: 8 additions & 17 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 11 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
"version": "1.0.2",
"description": "templated files used in npm CLI team oss projects",
"main": "lib/index.js",
"bin": {
"npm-template-check": "bin/npm-template-check.js"
},
"scripts": {
"prelint": "ln -sf ../../bin/npm-template-check.js node_modules/.bin/npm-template-check",
"lint": "eslint '**/*.js'",
"postlint": "npm-template-check",
"lintfix": "npm run lint -- --fix",
"preversion": "npm test",
"postversion": "npm publish",
Expand Down Expand Up @@ -34,11 +39,14 @@
"lib"
],
"devDependencies": {
"@npmcli/eslint-config": "^1.0.0",
"@npmcli/eslint-config": "*",
"eslint": "^7.32.0",
"eslint-plugin-node": "^11.1.0",
"spawk": "^1.7.1",
"tap": "*"
},
"peerDependencies": {
"@npmcli/eslint-config": "^1.0.0",
"tap": "^15.0.9"
},
"templateVersion": "1.0.1"
"templateVersion": "1.0.2"
}
32 changes: 0 additions & 32 deletions test/install.js

This file was deleted.

19 changes: 0 additions & 19 deletions test/postinstall.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
const { basename, dirname, join, normalize } = require('path')
const fs = require('@npmcli/fs')
const spawk = require('spawk')
const t = require('tap')

const TEMPLATE_VERSION = require('../package.json').version

const copyContents = require('../lib/content/index.js')
const installPackages = require('../lib/install.js')
const patchPackage = require('../lib/package.js')

spawk.preventUnmatched()

t.test('when npm_config_global is true, does nothing', async (t) => {
// this is set by virtue of running tests with npm, save it and remove it
const _env = process.env.npm_config_global
Expand Down Expand Up @@ -94,24 +90,9 @@ t.test('sets up a new project', async (t) => {
process.env.npm_config_local_prefix = _prefix
})

const uninstall = spawk.spawn('npm', (args) => {
return args[0] === 'uninstall' &&
installPackages.removeDeps.every((dep) => args.includes(dep))
}, { cwd: root })

const install = spawk.spawn('npm', (args) => {
return args[0] === 'install' &&
args[1] === '--save-dev' &&
installPackages.devDeps.every((dep) => args.includes(dep))
}, { cwd: root })

// t.mock instead of require so the cache doesn't interfere
await t.mock('../bin/postinstall.js')

// we mock the npm commands so this test doesn't take forever
t.ok(uninstall.called, 'ran uninstalls')
t.ok(install.called, 'ran installs')

// verify file contents were copied
const contents = await fs.readdir(root)
for (const file in copyContents.content) {
Expand Down

0 comments on commit d1380df

Please sign in to comment.