Skip to content

Commit b1965d6

Browse files
committed
fix!: refuse to pack when overrides apply to bundled packages
BREAKING CHANGE: npm pack and npm publish now error when a package's overrides apply to one or more of its bundled packages (bundledDependencies / bundleDependencies). Defining both fields is still allowed as long as no override actually targets a bundled package. To resolve the error, remove the affected entries from either overrides or the bundle.
1 parent 916cb4b commit b1965d6

2 files changed

Lines changed: 262 additions & 0 deletions

File tree

workspaces/libnpmpack/lib/index.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,42 @@ async function pack (spec = 'file:.', opts = {}) {
1414

1515
const manifest = await pacote.manifest(spec, { ...opts, Arborist, _isRoot: true })
1616

17+
if (spec.type === 'directory') {
18+
const hasBundled = manifest.bundleDependencies?.length > 0
19+
const hasOverrides = manifest.overrides
20+
&& typeof manifest.overrides === 'object'
21+
&& Object.keys(manifest.overrides).length > 0
22+
if (hasBundled && hasOverrides) {
23+
// Only refuse when an override rule actually applies to a package that is bundled by the root.
24+
// Overrides targeting dev dependencies or any package outside the bundled tree are harmless to consumers, because consumers do not apply the publishing package's overrides.
25+
// We rely on Arborist's own semantics (inBundle/inDepBundle/overridden) rather than reimplementing what npm-packlist/arborist already knows.
26+
const arb = new Arborist({ path: spec.fetchSpec })
27+
const tree = await arb.loadActual()
28+
const offenders = new Set()
29+
for (const node of tree.inventory.values()) {
30+
if (node.isRoot) {
31+
continue
32+
}
33+
// Only packages bundled by the root are at risk: nested dep-bundles are published as-is and arborist already treats them as immune to the root's overrides (see Edge#satisfiedBy).
34+
if (!node.inBundle || node.inDepBundle) {
35+
continue
36+
}
37+
if (node.overridden) {
38+
offenders.add(node.name)
39+
}
40+
}
41+
if (offenders.size) {
42+
const names = [...offenders].sort()
43+
const list = names.join(', ')
44+
const isOne = names.length === 1
45+
throw Object.assign(
46+
new Error(`Cannot pack or publish: "overrides" ${isOne ? 'affects a bundled package' : 'affect bundled packages'} (${list}). Consumers do not apply your package's overrides, so the published bundle will produce invalid dependency edges. Remove ${isOne ? 'this package' : 'these packages'} from "bundledDependencies"/"bundleDependencies" or from "overrides" before publishing.`),
47+
{ code: 'EBUNDLEOVERRIDE', packages: names }
48+
)
49+
}
50+
}
51+
}
52+
1753
const stdio = opts.foregroundScripts ? 'inherit' : 'pipe'
1854

1955
if (spec.type === 'directory' && !opts.ignoreScripts) {

workspaces/libnpmpack/test/index.js

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,3 +212,229 @@ t.test('doesn\'t run scripts when ignoreScripts === true', async t => {
212212
spawk.clean()
213213
})
214214
})
215+
216+
t.test('refuses to pack when overrides affect a bundled package', async t => {
217+
const testDir = t.testdir({
218+
'package.json': JSON.stringify({
219+
name: 'my-cool-pkg',
220+
version: '1.0.0',
221+
bundledDependencies: ['foo'],
222+
dependencies: { foo: '1.0.0' },
223+
overrides: { bar: '2.0.0' },
224+
}, null, 2),
225+
node_modules: {
226+
foo: {
227+
'package.json': JSON.stringify({
228+
name: 'foo',
229+
version: '1.0.0',
230+
dependencies: { bar: '^1.0.0' },
231+
}),
232+
node_modules: {
233+
bar: {
234+
'package.json': JSON.stringify({ name: 'bar', version: '2.0.0' }),
235+
},
236+
},
237+
},
238+
},
239+
})
240+
241+
const cwd = process.cwd()
242+
process.chdir(testDir)
243+
t.teardown(() => process.chdir(cwd))
244+
245+
await t.rejects(
246+
pack('file:.'),
247+
{
248+
code: 'EBUNDLEOVERRIDE',
249+
packages: ['bar'],
250+
message: /affects a bundled package \(bar\)/,
251+
},
252+
'throws EBUNDLEOVERRIDE listing the offending bundled package'
253+
)
254+
})
255+
256+
t.test('lists all offenders when multiple bundled packages are overridden', async t => {
257+
const testDir = t.testdir({
258+
'package.json': JSON.stringify({
259+
name: 'my-cool-pkg',
260+
version: '1.0.0',
261+
bundledDependencies: ['foo'],
262+
dependencies: { foo: '1.0.0' },
263+
overrides: { bar: '2.0.0', baz: '3.0.0' },
264+
}, null, 2),
265+
node_modules: {
266+
foo: {
267+
'package.json': JSON.stringify({
268+
name: 'foo',
269+
version: '1.0.0',
270+
dependencies: { bar: '^1.0.0', baz: '^1.0.0' },
271+
}),
272+
node_modules: {
273+
bar: {
274+
'package.json': JSON.stringify({ name: 'bar', version: '2.0.0' }),
275+
},
276+
baz: {
277+
'package.json': JSON.stringify({ name: 'baz', version: '3.0.0' }),
278+
},
279+
},
280+
},
281+
},
282+
})
283+
284+
const cwd = process.cwd()
285+
process.chdir(testDir)
286+
t.teardown(() => process.chdir(cwd))
287+
288+
await t.rejects(
289+
pack('file:.'),
290+
{
291+
code: 'EBUNDLEOVERRIDE',
292+
packages: ['bar', 'baz'],
293+
message: /affect bundled packages \(bar, baz\)/,
294+
},
295+
'lists every overridden bundled package and uses plural wording'
296+
)
297+
})
298+
299+
t.test('refuses to pack with bundleDependencies (alt spelling) + affected override', async t => {
300+
const testDir = t.testdir({
301+
'package.json': JSON.stringify({
302+
name: 'my-cool-pkg',
303+
version: '1.0.0',
304+
bundleDependencies: ['foo'],
305+
dependencies: { foo: '1.0.0' },
306+
overrides: { bar: '2.0.0' },
307+
}, null, 2),
308+
node_modules: {
309+
foo: {
310+
'package.json': JSON.stringify({
311+
name: 'foo',
312+
version: '1.0.0',
313+
dependencies: { bar: '^1.0.0' },
314+
}),
315+
node_modules: {
316+
bar: {
317+
'package.json': JSON.stringify({ name: 'bar', version: '2.0.0' }),
318+
},
319+
},
320+
},
321+
},
322+
})
323+
324+
const cwd = process.cwd()
325+
process.chdir(testDir)
326+
t.teardown(() => process.chdir(cwd))
327+
328+
await t.rejects(
329+
pack('file:.'),
330+
{ code: 'EBUNDLEOVERRIDE' },
331+
'throws EBUNDLEOVERRIDE with alternate bundleDependencies spelling'
332+
)
333+
})
334+
335+
t.test('packs when overrides target only a dev dependency (not bundled)', async t => {
336+
const testDir = t.testdir({
337+
'package.json': JSON.stringify({
338+
name: 'my-cool-pkg',
339+
version: '1.0.0',
340+
bundledDependencies: ['keep'],
341+
dependencies: { keep: '1.0.0' },
342+
devDependencies: { dev: '1.0.0' },
343+
overrides: { transdev: '2.0.0' },
344+
}, null, 2),
345+
node_modules: {
346+
keep: {
347+
'package.json': JSON.stringify({ name: 'keep', version: '1.0.0' }),
348+
},
349+
dev: {
350+
'package.json': JSON.stringify({
351+
name: 'dev',
352+
version: '1.0.0',
353+
dependencies: { transdev: '^1.0.0' },
354+
}),
355+
node_modules: {
356+
transdev: {
357+
'package.json': JSON.stringify({ name: 'transdev', version: '2.0.0' }),
358+
},
359+
},
360+
},
361+
},
362+
})
363+
364+
const cwd = process.cwd()
365+
process.chdir(testDir)
366+
t.teardown(() => process.chdir(cwd))
367+
368+
const tarball = await pack('file:.')
369+
t.ok(tarball, 'pack succeeds — overridden dev-only transitive dep is not in the bundle')
370+
})
371+
372+
t.test('packs when overrides target a package outside the bundled subtree', async t => {
373+
const testDir = t.testdir({
374+
'package.json': JSON.stringify({
375+
name: 'my-cool-pkg',
376+
version: '1.0.0',
377+
bundledDependencies: ['foo'],
378+
dependencies: { foo: '1.0.0', qux: '^1.0.0' },
379+
overrides: { baz: '2.0.0' },
380+
}, null, 2),
381+
node_modules: {
382+
foo: {
383+
'package.json': JSON.stringify({ name: 'foo', version: '1.0.0' }),
384+
},
385+
qux: {
386+
'package.json': JSON.stringify({
387+
name: 'qux',
388+
version: '1.0.0',
389+
dependencies: { baz: '^1.0.0' },
390+
}),
391+
node_modules: {
392+
baz: {
393+
'package.json': JSON.stringify({ name: 'baz', version: '2.0.0' }),
394+
},
395+
},
396+
},
397+
},
398+
})
399+
400+
const cwd = process.cwd()
401+
process.chdir(testDir)
402+
t.teardown(() => process.chdir(cwd))
403+
404+
const tarball = await pack('file:.')
405+
t.ok(tarball, 'pack succeeds — overridden package is not bundled')
406+
})
407+
408+
t.test('packs with only bundledDependencies (no overrides)', async t => {
409+
const testDir = t.testdir({
410+
'package.json': JSON.stringify({
411+
name: 'my-cool-pkg',
412+
version: '1.0.0',
413+
bundledDependencies: [],
414+
}, null, 2),
415+
})
416+
417+
const cwd = process.cwd()
418+
process.chdir(testDir)
419+
t.teardown(() => process.chdir(cwd))
420+
421+
const tarball = await pack('file:.')
422+
t.ok(tarball)
423+
})
424+
425+
t.test('packs with only overrides (no bundled)', async t => {
426+
const testDir = t.testdir({
427+
'package.json': JSON.stringify({
428+
name: 'my-cool-pkg',
429+
version: '1.0.0',
430+
overrides: { 'lru-cache': '6.0.0' },
431+
}, null, 2),
432+
})
433+
434+
const cwd = process.cwd()
435+
process.chdir(testDir)
436+
t.teardown(() => process.chdir(cwd))
437+
438+
const tarball = await pack('file:.')
439+
t.ok(tarball)
440+
})

0 commit comments

Comments
 (0)