yarn pack broken for bundledDependencies (and how to fix it)

Created on 27 Aug 2017  路  10Comments  路  Source: yarnpkg/yarn

Do you want to request a feature or report a bug?

Bug

What is the current behavior?
yarn pack is completely broken for usage with bundledDependencies because
1) const bundledDependencies = pkg.bundledDependencies,

the above seems to be a typo because yarn always normalizes the schema to "bundleDependencies" no matter if it's called bundledDependencies or bundleDependencies in package.json
so, unless I change it to pkg.bundleDependencies, it's always empty
2) there's a logic that creates pattern to exclude node_modules/$BUNDLED_DEPENDENCY but the logic later on in tar.pack ignore handler is also broken and these filters are not accounted for, thus not packaging /any/ bundled dependencies (not sure how to fix it)
3) tar.pack function does not follow symlinks which it should to properly pack the bundled dependencies (i.e. usage with npm link)
4) all bundled dependencies should be purged from package.json dependencies when packaging or otherwise npm install will try to resolve them and break

If the current behavior is a bug, please provide the steps to reproduce.

  1. Create a package nr 1
  2. Create a package nr 2
  3. npm link package nr2 in package nr1 and add entry to package.json like
    dependencies: {"package2":"../package2"}
  4. Try to use yarn pack
  5. Look at the resulting tar file

What is the expected behavior?

Please mention your node.js, yarn and operating system version.
yarn v0.27.5, Ubuntu

cat-bug help wanted triaged

Most helpful comment

2) removing node_modules from IGNORED array is enough
(it can be-readded if bundleDependencies is empty, for faster packaging)
3) I can't see an option to follow symlinks in tar-fs :/ (EDIT: there is an option, called deference=true ^^)

Also, for some reason fixing bundleDependencies -> bundledDependencies seems to break packaging package.json file. That's because there should be a filters.concat here https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/pack.js#L69

I'll try to work on a PR to fix all this stuff.

All 10 comments

related #2636

2) removing node_modules from IGNORED array is enough
(it can be-readded if bundleDependencies is empty, for faster packaging)
3) I can't see an option to follow symlinks in tar-fs :/ (EDIT: there is an option, called deference=true ^^)

Also, for some reason fixing bundleDependencies -> bundledDependencies seems to break packaging package.json file. That's because there should be a filters.concat here https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/pack.js#L69

I'll try to work on a PR to fix all this stuff.

@ninja- just curious on timing for this fix... little surprised yarn 1+ was promoted past rc with this bug in flight

Can i just point out that this issue is not limited to yarn pack? I am seeing a large lacking of support for bundledDependencies in general. I understand that the support is intended to be there - but somehow it bas fallen through the cracks.

For example, just install a dependency which utilizes bundledDependencies. Such as nyc - then just wait for the onslaught of false-negative yarn errors/warnings.

@ninja- Can we please raise the priority on this?

A failing test case has been merged for this: #4598

@hulkish I can create a PR fixing the part that I mentioned, I gave up trying to get a result that's matching $ npm pack...
especially if you have bundled dependencies that have other linked depdencies they need a package.json 'fix' etc. I don't know how it works exactly

Also we probably need additional logic for installing these via $ yarn install pkg.tar.gz or something?

@ninja- shouldn't this just be a matter of using realpath?

As i said there this option in tar-fs which does just that. Is that what you refer to?

But package.json of these deps need a change to install property so can't be directly copied.

@ninja- do you think we can close this one as a duplicate of #2636 since they look very similar to me?

@BYK ok then

Was this page helpful?
0 / 5 - 0 ratings