Skip to content

Commit

Permalink
fix(pack, publish): default foreground-scripts to true (#7158)
Browse files Browse the repository at this point in the history 
Fixes #6816
  • Loading branch information
@ljharb
ljharb committed Feb 27, 2024
1 parent a530215 commit 818957c
Show file tree
Hide file tree
Showing 8 changed files with 305 additions and 1 deletion.
3 changes: 3 additions & 0 deletions lib/commands/pack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class Pack extends BaseCommand {
for (const { arg, manifest } of manifests) {
const tarballData = await libpack(arg, {
...this.npm.flatOptions,
foregroundScripts: this.npm.config.isDefault('foreground-scripts')
? true
: this.npm.config.get('foreground-scripts'),
prefix: this.npm.localPrefix,
workspaces: this.workspacePaths,
})
Expand Down
3 changes: 3 additions & 0 deletions lib/commands/publish.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ class Publish extends BaseCommand {
// we pass dryRun: true to libnpmpack so it doesn't write the file to disk
const tarballData = await pack(spec, {
...opts,
foregroundScripts: this.npm.config.isDefault('foreground-scripts')
? true
: this.npm.config.get('foreground-scripts'),
dryRun: true,
prefix: this.npm.localPrefix,
workspaces: this.workspacePaths,
Expand Down
42 changes: 42 additions & 0 deletions tap-snapshots/test/lib/commands/pack.js.test.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,48 @@ Array [
]
`

exports[`test/lib/commands/pack.js TAP foreground-scripts can still be set to false > logs pack contents 1`] = `
Array [
undefined,
"package: test-fg-scripts@0.0.0",
undefined,
"110B package.json",
undefined,
String(
name: test-fg-scripts
version: 0.0.0
filename: test-fg-scripts-0.0.0.tgz
package size: {size}
unpacked size: 110 B
shasum: {sha}
integrity: {integrity}
total files: 1
),
"",
]
`

exports[`test/lib/commands/pack.js TAP foreground-scripts defaults to true > logs pack contents 1`] = `
Array [
undefined,
"package: test-fg-scripts@0.0.0",
undefined,
"110B package.json",
undefined,
String(
name: test-fg-scripts
version: 0.0.0
filename: test-fg-scripts-0.0.0.tgz
package size: {size}
unpacked size: 110 B
shasum: {sha}
integrity: {integrity}
total files: 1
),
"",
]
`

exports[`test/lib/commands/pack.js TAP should log output as valid json > logs pack contents 1`] = `
Array []
`
Expand Down
86 changes: 86 additions & 0 deletions tap-snapshots/test/lib/commands/publish.js.test.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,92 @@ Array [
]
`

exports[`test/lib/commands/publish.js TAP foreground-scripts can still be set to false > must match snapshot 1`] = `
Array [
Array [
"",
],
Array [
"",
"package: test-fg-scripts@0.0.0",
],
Array [
"=== Tarball Contents ===",
],
Array [
"",
"110B package.json",
],
Array [
"=== Tarball Details ===",
],
Array [
"",
String(
name: test-fg-scripts
version: 0.0.0
filename: test-fg-scripts-0.0.0.tgz
package size: {size}
unpacked size: 110 B
shasum: {sha}
integrity: {integrity}
total files: 1
),
],
Array [
"",
"",
],
Array [
"",
"Publishing to https://registry.npmjs.org/ with tag latest and default access (dry-run)",
],
]
`

exports[`test/lib/commands/publish.js TAP foreground-scripts defaults to true > must match snapshot 1`] = `
Array [
Array [
"",
],
Array [
"",
"package: test-fg-scripts@0.0.0",
],
Array [
"=== Tarball Contents ===",
],
Array [
"",
"110B package.json",
],
Array [
"=== Tarball Details ===",
],
Array [
"",
String(
name: test-fg-scripts
version: 0.0.0
filename: test-fg-scripts-0.0.0.tgz
package size: {size}
unpacked size: 110 B
shasum: {sha}
integrity: {integrity}
total files: 1
),
],
Array [
"",
"",
],
Array [
"",
"Publishing to https://registry.npmjs.org/ with tag latest and default access (dry-run)",
],
]
`

exports[`test/lib/commands/publish.js TAP has mTLS auth for scope configured registry > new package version 1`] = `
+ @npm/test-package@1.0.0
`
Expand Down
3 changes: 2 additions & 1 deletion tap-snapshots/test/lib/docs.js.test.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,8 @@ recommended that you do not use this option!
#### \`foreground-scripts\`
* Default: false
* Default: \`false\` unless when using \`npm pack\` or \`npm publish\` where it
defaults to \`true\`
* Type: Boolean
Run all build scripts (ie, \`preinstall\`, \`install\`, and \`postinstall\`)
Expand Down
81 changes: 81 additions & 0 deletions test/lib/commands/pack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,87 @@ t.test('dry run', async t => {
t.throws(() => fs.statSync(path.resolve(npm.prefix, filename)))
})

t.test('foreground-scripts defaults to true', async t => {
const { npm, outputs, logs } = await loadMockNpm(t, {
prefixDir: {
'package.json': JSON.stringify({
name: 'test-fg-scripts',
version: '0.0.0',
scripts: {
prepack: 'echo prepack!',
postpack: 'echo postpack!',
},
}
),
},
config: { 'dry-run': true },
})

/* eslint no-console: 0 */
// TODO: replace this with `const results = t.intercept(console, 'log')`
const log = console.log
t.teardown(() => {
console.log = log
})
const caughtLogs = []
console.log = (...args) => {
caughtLogs.push(args)
}
// end TODO

await npm.exec('pack', [])
const filename = 'test-fg-scripts-0.0.0.tgz'
t.same(
caughtLogs,
[
['\n> test-fg-scripts@0.0.0 prepack\n> echo prepack!\n'],
['\n> test-fg-scripts@0.0.0 postpack\n> echo postpack!\n'],
],
'prepack and postpack log to stdout')
t.strictSame(outputs, [[filename]])
t.matchSnapshot(logs.notice.map(([, m]) => m), 'logs pack contents')
t.throws(() => fs.statSync(path.resolve(npm.prefix, filename)))
})

t.test('foreground-scripts can still be set to false', async t => {
const { npm, outputs, logs } = await loadMockNpm(t, {
prefixDir: {
'package.json': JSON.stringify({
name: 'test-fg-scripts',
version: '0.0.0',
scripts: {
prepack: 'echo prepack!',
postpack: 'echo postpack!',
},
}
),
},
config: { 'dry-run': true, 'foreground-scripts': false },
})

/* eslint no-console: 0 */
// TODO: replace this with `const results = t.intercept(console, 'log')`
const log = console.log
t.teardown(() => {
console.log = log
})
const caughtLogs = []
console.log = (...args) => {
caughtLogs.push(args)
}
// end TODO

await npm.exec('pack', [])
const filename = 'test-fg-scripts-0.0.0.tgz'
t.same(
caughtLogs,
[],
'prepack and postpack do not log to stdout')
t.strictSame(outputs, [[filename]])
t.matchSnapshot(logs.notice.map(([, m]) => m), 'logs pack contents')
t.throws(() => fs.statSync(path.resolve(npm.prefix, filename)))
})

t.test('invalid packument', async t => {
const { npm, outputs } = await loadMockNpm(t, {
prefixDir: {
Expand Down
86 changes: 86 additions & 0 deletions test/lib/commands/publish.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,92 @@ t.test('dry-run', async t => {
t.matchSnapshot(logs.notice)
})

t.test('foreground-scripts defaults to true', async t => {
const { joinedOutput, npm, logs } = await loadMockNpm(t, {
config: {
'dry-run': true,
...auth,
},
prefixDir: {
'package.json': JSON.stringify({
name: 'test-fg-scripts',
version: '0.0.0',
scripts: {
prepack: 'echo prepack!',
postpack: 'echo postpack!',
},
}
),
},
})

/* eslint no-console: 0 */
// TODO: replace this with `const results = t.intercept(console, 'log')`
const log = console.log
t.teardown(() => {
console.log = log
})
const caughtLogs = []
console.log = (...args) => {
caughtLogs.push(args)
}
// end TODO

await npm.exec('publish', [])
t.equal(joinedOutput(), `+ test-fg-scripts@0.0.0`)
t.matchSnapshot(logs.notice)

t.same(
caughtLogs,
[
['\n> test-fg-scripts@0.0.0 prepack\n> echo prepack!\n'],
['\n> test-fg-scripts@0.0.0 postpack\n> echo postpack!\n'],
],
'prepack and postpack log to stdout')
})

t.test('foreground-scripts can still be set to false', async t => {
const { joinedOutput, npm, logs } = await loadMockNpm(t, {
config: {
'dry-run': true,
'foreground-scripts': false,
...auth,
},
prefixDir: {
'package.json': JSON.stringify({
name: 'test-fg-scripts',
version: '0.0.0',
scripts: {
prepack: 'echo prepack!',
postpack: 'echo postpack!',
},
}
),
},
})

/* eslint no-console: 0 */
// TODO: replace this with `const results = t.intercept(console, 'log')`
const log = console.log
t.teardown(() => {
console.log = log
})
const caughtLogs = []
console.log = (...args) => {
caughtLogs.push(args)
}
// end TODO

await npm.exec('publish', [])
t.equal(joinedOutput(), `+ test-fg-scripts@0.0.0`)
t.matchSnapshot(logs.notice)

t.same(
caughtLogs,
[],
'prepack and postpack do not log to stdout')
})

t.test('shows usage with wrong set of arguments', async t => {
const { publish } = await loadMockNpm(t, { command: 'publish' })
await t.rejects(publish.exec(['a', 'b', 'c']), publish.usage)
Expand Down
2 changes: 2 additions & 0 deletions workspaces/config/lib/definitions/definitions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,8 @@ define('force', {

define('foreground-scripts', {
default: false,
defaultDescription: `\`false\` unless when using \`npm pack\` or \`npm publish\` where it
defaults to \`true\``,
type: Boolean,
description: `
Run all build scripts (ie, \`preinstall\`, \`install\`, and
Expand Down

0 comments on commit 818957c

Please sign in to comment.