amireh / happypack

Happiness in the form of faster webpack build times.
MIT License
4.24k stars 124 forks source link

Ignore cache-loader directory when publishing #205

Closed filipesilva closed 6 years ago

filipesilva commented 6 years ago

Currently .cache-loader is being published, with some local data from tests: https://unpkg.com/happypack@4.0.0/.cache-loader/

codecov-io commented 6 years ago

Codecov Report

Merging #205 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #205   +/-   ##
=======================================
  Coverage   94.53%   94.53%           
=======================================
  Files          25       25           
  Lines         713      713           
=======================================
  Hits          674      674           
  Misses         39       39

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f153db5...88f36c3. Read the comment docs.

amireh commented 6 years ago

Nice catch, thanks.

Since something similar happened in the past, what do you think about adopting a whitelist approach instead? IIRC package.json allows a files listing that can be used for this purpose.

filipesilva commented 6 years ago

That seems reasonable as well. In that case .npm-ignore would be deleted and files would look like this:

"files": [
  "lib",
  "loader.js",
  "CHANGELOG.md",
  "README.md"
],

This represents a problem with the /lib/__tests__ ignore rule though. To ignore it, a new ./lib/.npmignore will need to be added with just /lib/__tests__, because of this:

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.

WDYT?

amireh commented 6 years ago

I'm thinking maybe we can move the tests outside of lib altogether, but I can do that in a separate patchset.

The following looks good to me (dirs first then files, in alphabetical order):

"files": [
    "lib",
    "CHANGELOG.md",
    "LICENSE",
    "loader.js",
    "README.md"
  ],

I just tested that and it tars at 25K with the proper files (assuming you use .npmignore inside lib/.)

filipesilva commented 6 years ago

@amireh added the files array as you listed, and moved .npmignore with only the tests entry.

amireh commented 6 years ago

Looks good to me, gonna pull it down to double-check.

amireh commented 6 years ago

@filipesilva thanks, will publish a new revision shortly!

filipesilva commented 6 years ago

Coolio, thanks for the quick turnaround!

amireh commented 6 years ago

Version 4.0.1 is now up on npm with this change