Add files entry to package.json by mroderick · Pull Request #26 · sinonjs/nise

mroderick · 2018-01-28T20:40:13Z

This prevents unnecessary files to be published to the npm registry, saving bandwidth and speeding up installs for everyone.

Before

du -h nise-1.2.0.tgz
272K	nise-1.2.0.tgz

After

du -h nise-1.2.0.tgz
240K	nise-1.2.0.tgz

With ~1.2M downloads/month, this will save 1,200,000 * 32,000 / 1024^3 = ~36GB/month

How to verify

Check out this branch
npm pack
Expand the package
Observe that nise.js is in root, and that lib/ has no tests in it, but still contains all the source files.

This prevents unnecessary files to be published to the npm registry, saving bandwidth and speeding up installs for everyone.

mantoni · 2018-01-29T07:43:27Z

package.json

  },
+  "files": [
+    "nise.js",
+    "lib/**/*[^test].js"


Ha! Didn't know you could get this fancy in the files section 😆

mantoni · 2018-01-29T07:43:56Z

👍

mroderick · 2018-01-29T08:32:33Z

This has been published as nise@1.2.1

Flarna · 2018-01-29T08:50:58Z

Not sure if this PR is the root cause but it seems in nise@1.2.1 some files in nise/lib/event are missing resulting in Error: Cannot find module './event' during using sinon.

fatso83 · 2018-01-29T08:58:10Z

I was going to say this
~@Flarna thanks for reporting. that might all be well and true, but if you use nise implicitly through a sinon dependency I doubt this is the root cause - unless you use a old (< 7) Node version that doesn't support package-lock.json.

~~The reason is that we use package-lock.json to lock down dependencies~~

but then I remembered this from the docs:

One key detail about package-lock.json is that it cannot be published, and it will be ignored if found in any place other than the toplevel package.

So thanks for reporting, and we will look into it 👍

Flarna · 2018-01-29T09:12:59Z

I think the regex is not correct as [^test] doesn't mean everything except test in the name.
You could try to use a .npmignore file instead the files section.

fatso83 · 2018-01-29T09:14:17Z

This seems fine, @Flarna

 get-npm-file nise@1.2.1
package/package.json
package/LICENSE
package/nise.js
package/README.md
package/lib/configure-logger/index.js
package/lib/event/index.js
package/lib/fake-server/fake-server-with-clock.js
package/lib/fake-server/index.js
package/lib/fake-xhr/blob.js
package/lib/fake-xhr/index.js
package/lib/index.js

Script from here

Xylem · 2018-01-29T09:17:36Z

@fatso83 - the listing is missing all files from lib/event directory, except for index.js, for example.

fatso83 · 2018-01-29T09:18:31Z

Yeah, something isn't right, @mroderick:

Reproduction


carlerik at diffia9350 in /tmp 
$ cd test/

carlerik at diffia9350 in /tmp/test 
$ npm init
This utility will walk you through creating a package.json file.
It only covers the most common items, and tries to guess sensible defaults.

See `npm help json` for definitive documentation on these fields
and exactly what they do.

Use `npm install <pkg>` afterwards to install a package and
save it as a dependency in the package.json file.

Press ^C at any time to quit.
package name: (test) 
version: (1.0.0) 
description: 
entry point: (index.js) 
test command: 
git repository: 
keywords: 
author: 
license: (ISC) 
About to write to /tmp/test/package.json:

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}


Is this ok? (yes) 

carlerik at diffia9350 in /tmp/test 
$ npm i nise@1.2.1
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN test@1.0.0 No description
npm WARN test@1.0.0 No repository field.

+ nise@1.2.1
added 8 packages in 0.934s

carlerik at diffia9350 in /tmp/test 
$ node -e 'require("nise")'
module.js:529
    throw err;
    ^

Error: Cannot find module './event'
    at Function.Module._resolveFilename (module.js:527:15)
    at Function.Module._load (module.js:476:23)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/tmp/test/node_modules/nise/lib/event/index.js:4:12)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)

fatso83 · 2018-01-29T09:27:21Z

Fix

diff --git a/package.json b/package.json
index 5301a22..316e6b8 100644
--- a/package.json
+++ b/package.json
@@ -36,7 +36,8 @@
   },
   "files": [
     "nise.js",
-    "lib/**/*[^test].js"
+    "lib/**/*.js",
+    "lib/**/!*test.js"
   ],
   "devDependencies": {
     "browserify": "^14.4.0",

$ npm pack

$ tar tzf nise-1.2.1.tgz 
package/package.json
package/LICENSE
package/nise.js
package/README.md
package/lib/configure-logger/index.js
package/lib/configure-logger/index.test.js
package/lib/event/custom-event.js
package/lib/event/event-target.js
package/lib/event/event.js
package/lib/event/index.js
package/lib/event/index.test.js
package/lib/event/progress-event.js
package/lib/fake-server/fake-server-with-clock.js
package/lib/fake-server/fake-server-with-clock.test.js
package/lib/fake-server/format.js
package/lib/fake-server/format.test.js
package/lib/fake-server/index.js
package/lib/fake-server/index.test.js
package/lib/fake-xhr/blob.js
package/lib/fake-xhr/index.js
package/lib/fake-xhr/index.test.js
package/lib/index.js
package/lib/index.test.js

Xylem · 2018-01-29T09:28:03Z

Would it be possible to deprecate the broken version?

fatso83 · 2018-01-29T09:30:04Z

@Xylem Done

Add files entry to package.json

4e5ee2b

This prevents unnecessary files to be published to the npm registry, saving bandwidth and speeding up installs for everyone.

mroderick requested review from fatso83 and mantoni January 28, 2018 20:40

mantoni reviewed Jan 29, 2018

View reviewed changes

package.json

},

"files": [

"nise.js",

"lib/**/*[^test].js"

Copy link

Member

mantoni Jan 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha! Didn't know you could get this fancy in the files section 😆

mantoni merged commit 8b92792 into sinonjs:master Jan 29, 2018

mroderick deleted the add-files-entry-to-package.json branch January 29, 2018 08:32

Flarna mentioned this pull request Jan 29, 2018

Error: Cannot find module './event' #27

Closed

fatso83 mentioned this pull request Jan 29, 2018

Fix for version glob error in #26 #29

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add files entry to package.json#26

Add files entry to package.json#26
mantoni merged 1 commit intosinonjs:masterfrom
mroderick:add-files-entry-to-package.json

mroderick commented Jan 28, 2018 •

edited

Loading

Uh oh!

mantoni Jan 29, 2018

Uh oh!

mantoni commented Jan 29, 2018

Uh oh!

mroderick commented Jan 29, 2018

Uh oh!

Flarna commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018

Uh oh!

Flarna commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018

Uh oh!

Xylem commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018 •

edited

Loading

Uh oh!

fatso83 commented Jan 29, 2018 •

edited

Loading

Uh oh!

Xylem commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

mroderick commented Jan 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before

After

How to verify

Uh oh!

mantoni Jan 29, 2018

Choose a reason for hiding this comment

Uh oh!

mantoni commented Jan 29, 2018

Uh oh!

mroderick commented Jan 29, 2018

Uh oh!

Flarna commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018

Uh oh!

Flarna commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018

Uh oh!

Xylem commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fatso83 commented Jan 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Xylem commented Jan 29, 2018

Uh oh!

fatso83 commented Jan 29, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mroderick commented Jan 28, 2018 •

edited

Loading

fatso83 commented Jan 29, 2018 •

edited

Loading

fatso83 commented Jan 29, 2018 •

edited

Loading