yarnpkg / yarn

The 1.x line is frozen - features and bugfixes now happen on https://github.com/yarnpkg/berry
https://classic.yarnpkg.com
Other
41.39k stars 2.72k forks source link

Yarn pack is ignoring .npmignore files #685

Open paul-sachs opened 7 years ago

paul-sachs commented 7 years ago

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

Bug

What is the current behavior?

If I specify a .npmignore with a bunch of files that should NOT be published along with the package, a bunch of files seem to be included no matter what and some aren't included at all.

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

  1. Take existing project. My project has a lib directory. Create a file `.npmignore'
  2. Add a few directories that you want to exclude from the package. eg
src
node_modules
  1. Run yarn pack
  2. Run tar -ztvf <package>

Notice that the package contains README.md files that were specified in node_modules AND in src. It also does not include the directory lib.

What is the expected behavior?

node_modules and src should not exist in the archive. lib should exist in the archive.

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

OS X: 10.11.6 node: 5.5.0 yarn: 0.15.1

sebmck commented 7 years ago

Thanks for the report! Can you post the package.json you have? (Feel free to censor sensitive parts) We have some magic to handle the files and bundledDependencies fields which may be effecting this.

paul-sachs commented 7 years ago

Sure.

{
  "name": "patternity",
  "version": "2.0.0",
  "description": "Patternity is the pattern library and style guide for all Influitive apps",
  "main": "index.js",
  "scripts": {
    "test": "tape-babel-css-modules \"src/**/*/*.test.js\" | faucet && karma start",
    "test-circleci": "tape-babel-css-modules \"src/**/*/*.test.js\" | faucet && karma start --single-run --log-level error",
    "karma-test": "karma start",
    "tape-watch": "watch 'clear && tape-babel-css-modules \"src/**/*/*.test.js\" | faucet && exit 0' src -d -u",
    "coverage": "babel-node node_modules/.bin/isparta cover src/**/*/*.test.js | tap-spec",
    "babel": "babel {src,infl-components-src} --out-dir lib --ignore *.test.js && gulp copy-lib-styles",
    "babel-watch": "babel {src,infl-components-src} -w --out-dir lib & gulp watch-scss",
    "testbed": "node ./webpacktestbed/devServer.js",
    "postinstall": "npm run babel",
    "create": "gulp create-component",
    "lint": "eslint ./src; exit 0",
    "lint:check": "eslint ./src",
    "githooks": "cd .git/hooks && ln -s ../../.githooks/commit-msg ./commit-msg && cd -",
    "newcomp": "babel-node .newcomponent.js"
  },
  "repository": {
    "type": "git",
    "url": "https://github.com/influitive/patternity.git"
  },
  "keywords": [
    "Pattern",
    "Lab",
    "Style",
    "Guide",
    "Infl-Styles",
    "Infl-Components"
  ],
  "author": "Influitive Team",
  "license": "ISC",
  "bugs": {
    "url": "https://github.com/influitive/patternity/issues"
  },
  "homepage": "https://github.com/influitive/patternity",
  "browserify": {
    "transform": [
      "reactify"
    ]
  },
  "devDependencies": {
    "autoprefixer": "^6.3.1",
    "babel-eslint": "^4.0.5",
    "babel-tape-runner": "^1.2.0",
    "babelify": "^6.3.0",
    "browserify": "^8.1.3",
    "browserify-single-file": "^0.3.0",
    "chai": "^3.5.0",
    "chalk": "^1.1.1",
    "css-loader": "^0.19.0",
    "css-modulesify": "^0.17.0",
    "eslint": "^1.2.0",
    "eslint-plugin-react": "^3.2.2",
    "express": "^4.13.3",
    "extract-text-webpack-plugin": "^0.8.2",
    "faucet": "0.0.1",
    "file-loader": "^0.8.5",
    "html-webpack-plugin": "^2.8.0",
    "infl-icons": "git://github.com/influitive/infl-icons.git",
    "is-dir": "^1.0.0",
    "isparta": "^3.1.0",
    "jasmine-core": "^2.2.0",
    "json-loader": "^0.5.4",
    "karma": "^0.12.31",
    "karma-browserify": "^3.0.1",
    "karma-chrome-launcher": "^0.1.7",
    "karma-jasmine": "^0.3.5",
    "karma-mocha": "^0.1.10",
    "karma-phantomjs-launcher": "^1.0.0",
    "lost": "^6.7.1",
    "mocha": "^2.4.4",
    "postcss": "^5.0.16",
    "postcss-loader": "^0.8.0",
    "postcss-modules-extract-imports": "^1.0.0",
    "postcss-modules-local-by-default": "^1.0.1",
    "postcss-modules-scope": "^1.0.0",
    "precss": "^1.4.0",
    "react": "^0.14.3",
    "react-addons-test-utils": "^0.14.3",
    "react-dom": "^0.14.3",
    "react-hot-loader": "^1.2.9",
    "react-transform-catch-errors": "^1.0.0",
    "react-transform-hmr": "^1.0.0",
    "reactify": "^1.0.0",
    "sass-loader": "^2.0.1",
    "sinon": "^1.17.3",
    "sinon-chai": "^2.8.0",
    "skin-deep": "^0.13.0",
    "strictify": "^0.2.0",
    "style-loader": "^0.12.4",
    "tape": "^4.2.2",
    "tape-babel-css-modules": "git://github.com/justinsisley/tape-babel-css-modules.git",
    "tape-catch": "^1.0.4",
    "testling": "^1.7.1",
    "url-loader": "^0.5.7",
    "watchify": "^3.5.0",
    "webpack": "^1.12.12",
    "webpack-dev-middleware": "^1.2.0",
    "webpack-dev-server": "^1.10.1",
    "webpack-hot-middleware": "^2.2.0"
  },
  "dependencies": {
    "babel-cli": "^6.7.7",
    "babel-core": "^6.7.7",
    "babel-loader": "^6.2.4",
    "babel-plugin-add-module-exports": "^0.2.1",
    "babel-plugin-react-display-name": "^2.0.0",
    "babel-preset-es2015": "^6.6.0",
    "babel-preset-react": "^6.5.0",
    "babel-preset-stage-0": "^6.5.0",
    "classnames": "^1.1.4",
    "color": "^0.10.1",
    "es5-shim": "^4.1.0",
    "gulp": "^3.9.0",
    "gulp-batch": "^1.0.5",
    "gulp-debug": "^2.1.2",
    "gulp-flatten": "^0.2.0",
    "gulp-header": "^1.7.1",
    "gulp-notify": "^2.2.0",
    "gulp-postcss": "^6.0.0",
    "gulp-replace": "^0.5.4",
    "gulp-watch": "^4.3.5",
    "jquery": "^1.9.1",
    "jss": "^3.2.0",
    "jss-nested": "^1.0.1",
    "lodash": "^3.7.0",
    "lost": "^6.7.1",
    "moment": "^2.11.2",
    "node-bourbon": "~1.2.3",
    "node-neat": "^1.7.2",
    "node-sass": "^3.2.0",
    "phantomjs-prebuilt": "^2.1.7",
    "postcss": "^5.0.14",
    "postcss-media-minmax": "^2.1.1",
    "postcss-nested": "^1.0.0",
    "postcss-scss": "^0.1.2",
    "quill": "^0.20.1",
    "react-color": "1.3.6",
    "react-datetime": "^2.0.3",
    "react-jss": "^2.0.1",
    "react-quill": "^0.4.0",
    "react-tether": "^0.5.2",
    "redbox-react": "^1.1.0",
    "rucksack-css": "^0.8.2",
    "tap-bail": "0.0.0",
    "tap-pessimist": "^1.0.1",
    "tap-spec": "^4.1.0"
  },
  "peerDependencies": {
    "react": "^0.14.3"
  }
}

It's a little bit of a mess. It's a legacy package.

ghost commented 7 years ago

I assume the reason the lib folder is not in there is because it is in the .gitignore (patternity .gitignore). This is related to https://github.com/yarnpkg/yarn/issues/754 where the issue defined is that all ignore files are so to say merged together (I tried to explain the problem deeper in that issue).

Ciantic commented 7 years ago

I have multiple packages with package.json syntax file../other-package, and if I yarn install one of them, it ignores the .npmignore, thus copying "node_modules" several times causing whole lot of unneccessary copying. Is this related? I do not have files defined in package.json, only the .npmignore file.

P.S. I tried with files inclusion also, it doesn't seem to work either. It always keeps copying the node_modules of other-package, causing the installation to bloat.

eurochriskelly commented 7 years ago

+1 @Ciantic

Developing local packages can be a problem because of this For example, if the package is on git, the entire .git folder gets installed, even though it's in .npmignore (also tried .yarnignore), but yarn can't cope with it. No further packages can be installed using yarn. A workaround is to use npm install file://path/to/file for local modules but I'd rather have just one dependency manager.

yarn 0.20.3, windows 10

caesarsol commented 7 years ago

Isn't this an exact duplicate of https://github.com/yarnpkg/yarn/issues/754?

DylanVann commented 7 years ago

This makes testing React Native modules a hassle. If you have project/example with a dependency on "project": "../" it will copy project/node_modules into project/example/node_modules which will generally break the React Native packager due to @providesModule conflicts. Hopefully @providesModule will be deprecated soon (https://github.com/facebook/react/issues/6336) but this is an issue now.

Ciantic commented 7 years ago

I think this should be renamed, it's not just "yarn pack" it's also "yarn install", which is far more common.

caesarsol commented 7 years ago

I guess all yarn is ignoring .npmignore at this point?

This may be by design, in which case I'd say it's quite a strange decision!

gaguirre commented 7 years ago

I'm facing the same issue and it's really annoying. Currently I'm using yarn, removing my local modules from node_modules and then installing them using npm 😞

GAumala commented 7 years ago

It seems yarn is not reading .npmignore in the yarn publish command. I can publish my modules with npm, but not with yarn.

fvgs commented 7 years ago

I just tested yarn pack (which is the underlying command used by yarn publish) for packaging a module and encountered the same results as others. yarn pack does not take into account the contents of .npmignore. The same goes for .yarnignore. Thus, files which should be ignored are being packaged and published.

I'm using the latest v0.24.6 and this behavior is unexpected and inconsistent with npm's behavior. @kittens @bestander @Daniel15 could you please comment on what the expected behavior is and what the barriers to resolving this are?

GAumala commented 7 years ago

The root of the issue is that yarn gives the same priority to both .npmignore and .gitignore while npm does not. I have sent a PR with a patch. In the meantime, you could temporarily delete your .gitignore before you run pack or publish as a workaround.

fvgs commented 7 years ago

@GAumala what version of yarn are you using? The issue I'm encountering goes further than what you're describing, as none of .npmignore, .gitignore, or .yarnignore seem to have an impact on the files yarn pack includes.

GAumala commented 7 years ago

My system version is 0.24.5. My patched version which has commits up to today's morning works fine for my use case while system version does not.

In my use case I have a single .gitignore at project root which says ignore all .js files and a single .npmignore, also at root, which says ignore all .ts files. yarn 0.24.5 ignores both .js and .ts files, while my patched version only ignores .ts files as expected.

Maybe your .gitignore and .npmignore are cancelling each other out, or it could be a completely unrelated issue.

rardoz commented 7 years ago

I am also seeing this issue. I can validate that moving the .gitignore, running the yarn pack command, and then putting it back does indeed work for now. Make sure to ignore the .gitbackup file or that will come through in the package.

mv .gitignore .gitbackup && yarn pack && mv .gitbackup .gitignore

bestander commented 7 years ago

That should not be hard to fix, anyone volunteers? The code is here https://github.com/yarnpkg/yarn/blob/master/src/cli/commands/pack.js

bestander commented 7 years ago

fixed via https://github.com/yarnpkg/yarn/pull/3538

ZebraFlesh commented 6 years ago

I'm still seeing this on

yarn: 1.0.2 node: 6.9.1 OS: Windows 10 Pro (10.0.10586 Build 10586)

OmgImAlexis commented 6 years ago

Same here with yarn v1.3.2.

.npmignore

.git*
.DS_Store
testlocales/
test/
locales/
localestowrite/
localesmakeplural/
i18n-cov.js
coverage.html
examples/

.gitignore

node_modules/
testlocales/
coverage/
.DS_Store
/npm-debug.log
/i18n-cov.js
/coverage.html
defaultlocales
localestowrite
localesmakeplural

I ran yarn pack and npm pack for the tests.

➜  i18n-node git:(master) ✗ ls -1a yarn-package
.
..
LICENSE
README.md
examples
index.js
lib
package.json
➜  i18n-node git:(master) ✗ ls -1a npm-package
.
..
LICENSE
README.md
index.js
lib
package.json
➜  i18n-node git:(master) ✗ cat package.json 
{
  "name": "i18n",
  "description": "lightweight translation module with dynamic json storage",
  "version": "0.8.3",
  "homepage": "http://github.com/mashpie/i18n-node",
  "repository": {
    "type": "git",
    "url": "http://github.com/mashpie/i18n-node.git"
  },
  "author": "Marcus Spiegel <marcus.spiegel@gmail.com>",
  "main": "./index",
  "keywords": [
    "template",
    "i18n",
    "l10n"
  ],
  "directories": {
    "lib": "lib"
  },
  "files": [
      "index.js",
      "lib/"
  ],
  "dependencies": {
    "debug": "*",
    "make-plural": "^3.0.3",
    "math-interval-parser": "^1.1.0",
    "messageformat": "^0.3.1",
    "mustache": "*",
    "sprintf-js": ">=1.0.3"
  },
  "devDependencies": {
    "async": "*",
    "cookie-parser": "^1.4.1",
    "express": "^4.13.4",
    "jshint": "*",
    "mocha": "*",
    "should": "*",
    "sinon": "*",
    "url": "^0.11.0",
    "zombie": "*"
  },
  "engines": {
    "node": ">=0.10.0"
  },
  "scripts": {
    "jshint": "jshint --verbose .",
    "test": "npm run jshint && make test",
    "test-ci": "npm run jshint && istanbul cover ./node_modules/mocha/bin/_mocha"
  },
  "license": "MIT"
}

PING: @bestander

GAumala commented 6 years ago

I think yarn pack does not read .npmignore at all if you specify your files with files attribute in package.json. If anyone else still has this issue please post your package.json as well.

OmgImAlexis commented 6 years ago

@GAumala added my package.json above.

GAumala commented 6 years ago

Hey, thanks for adding that. It seems I was correct. Yarn gives files a higher priority that .npmignore. Incluiding files correctly is a bit tricky, here's what npm says about files and .npmignore:

You can also provide a .npmignore file in the root of your package or in subdirectories, which will keep files from being included. At the root of your package it will not override the "files" field, but in subdirectories it will.

What yarn currently does when you specify files is to add regexes that ignore everything except what 's inside of files. This is totally wrong according to the npm docs, but I'm not sure yet about how to fix it.

OmgImAlexis commented 6 years ago

Does this at least call for the issue to be reopened?

dan-kez commented 6 years ago

I am having the same issue as those above. files is given a higher priority over .npmignore

bestander commented 6 years ago

Let's open a new issue so that it is easier to track what is fixed and what needs to be done. The quickest way is to send a PR, pack command source code is very simple. Ping me when a review is needed

pratyoosh commented 6 years ago

I can replicate this issue as far as 1.5.1, the only thing worked was to temporarily delete .gitignore & then run "yarn package", worthwhile to note that i didn't had any files declared in my package.json

bestander commented 6 years ago

Turns out it is of a feature, pack command does not pack files listed in .gitignore, .npmignore and .yarnignore.

I understand the idea behind this - so that people don't pack build artifacts when they publish packages.

I added some code to walk around this for offline mirror feature here: https://github.com/yarnpkg/yarn/pull/5793.

But I am not sure if this should be the default behavior for pack command.

I can open the issue to allow the discussion.

Vadorequest commented 5 years ago

I'd just like to clarify a few things:

I recommend to read https://medium.com/@jdxcode/for-the-love-of-god-dont-use-npmignore-f93c08909d8d

SeijiSuenaga commented 5 years ago

@Vadorequest To clarify further, what you described is NPM's behavior. With Yarn, files whitelist rules always override .npmignore blacklist rules.

Others have said that Yarn doesn't read .npmignore at all if files is defined, but that's actually not true. When files is defined, blacklist rules don't work, but whitelist rules in .npmignore such as !do-not-ignore-me.js do.

The code that defines this behavior is in the sortFilter() utility function. As soon as the function finds a matching whitelist rule, it skips all other rules. I have a feeling this is because Yarn wants to always include certain files like package.json, but that should be achievable without breaking user-defined blacklist rules.

I think user-defined rules should be processed first, allowing blacklist rules to override whitelist rules. Then the built-in whitelist for package.json, etc. should override any user-defined rules affecting those files.

johanblumenberg commented 3 years ago

It seems everyone is in agreement that using a whitelist is much better than using a blacklist. There are security concerns with using a blacklist because you might accidentally forget to blacklist sensitive files.

This issue is doing two things:

Welcome to yarn. All your credentials are leaked.

obermillerk commented 2 years ago

I tried every solution in this thread while using yarn 1.22.17, to no avail. I had to upgrade yarn from v1 for it to work using the package.json files option. So v1 seemingly is forever broken in this regard.

leoj3n commented 2 years ago

I tried every solution in this thread while using yarn 1.22.17, to no avail. I had to upgrade yarn from v1 for it to work using the package.json files option. So v1 seemingly is forever broken in this regard.

Can confirm same experience here with 1.19.1 and 1.22.17 when using yarn add ../local-package.

Tried .npmignore as well as files entry in package.json, neither stopped node_modules from being included.

Pierstoval commented 2 months ago

Coming here years later and I can say I have exactly the same issues. This enforces me to use NPM which I didn't want in the first place at all.

callmeberzerker commented 2 months ago

Use pnpm like all the modern JS libs have been for the past 2+ years. 👌

Daniel15 commented 2 months ago

Yarn v1 is not under active development any more. If you want this fixed, you'll either have to contribute a fix yourself, or report a bug for Yarn v4 if this is still an issue in newer versions.