[Bug?]: `yarn pack` always ignores nested `node_modules`

mhassan1 commented 2 years ago

Self-service

[X] I'd be willing to implement a fix

Describe the bug

yarn pack always ignores nested node_modules directories, which is unexpected and inconsistent with npm pack.

To reproduce

const { mkdirSync, writeFileSync } = require('fs')

await packageJsonAndInstall({
  files: [ 'dist' ]
})

mkdirSync('dist/node_modules', { recursive: true })
writeFileSync('dist/file1.js', '')
writeFileSync('dist/node_modules/file2.js', '')

const output = await yarn(`pack`, `--dry-run`)

expect(output).toContain('dist/file1.js')
expect(output).toContain('dist/node_modules/file2.js')

Environment

System:
  OS: macOS 12.2.1
  CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
Binaries:
  Node: 16.14.0 - /private/var/folders/82/cn2lb31s5hd572v7x5wkc1vc0000gq/T/xfs-063d52a6/node
  Yarn: 3.2.0 - /private/var/folders/82/cn2lb31s5hd572v7x5wkc1vc0000gq/T/xfs-063d52a6/yarn
  npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm

Additional context

I noticed this bug(?) when I was using @rollup/plugin-typescript with PnP. That plugin uses the require.resolve('tslib') path as the place to include tslib in the dist, so I ended up with a dist that looks like this:

dist
  .yarn
    berry
      cache
        tslib-npm-2.3.1-e9a172c41d
          node_modules
            tslib

Even though it's a weird dist, it's a valid dist, but yarn pack (with files: ["dist"] in package.json) excludes the files inside node_modules because of this line: https://github.com/yarnpkg/berry/blob/2460dba707187d649048ab96bcad2e1e58ed5558/packages/plugin-pack/sources/packUtils.ts#L32

I don't see a way to tell yarn pack to include the nested node_modules using the files configuration in package.json; npm pack includes everything in the dist directory, including the nested node_modules.

I found a workaround for my situation (to bundle tslib separately from the plugin so I can control the path), but I wanted to raise this as a bug(?), at least so someone else who runs into this might benefit from the writeup.

I know it's not a goal for yarn pack to have parity with npm pack, but I was surprised by this behavior of yarn pack.

yarnbot commented 2 years ago

This issue reproduces on master:

Error: expect(received).toContain(expected) // indexOf

Expected substring: "dist/node_modules/file2.js"
Received string:    "➤ YN0000: dist/file1.js
➤ YN0000: package.json
➤ YN0000: Done in 0s 35ms
"
    at module.exports (evalmachine.<anonymous>:15:16)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:57:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:18:16)
    at async executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:25:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.3-558f52b79f-286d94b96d.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:26:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-4061026d74.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-4061026d74.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-4061026d74.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)

vegerot commented 1 week ago

👋any update? Would you accept a patch that fixes this bug?

yarnpkg / berry