Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slim down npm package #1589

Merged
merged 1 commit into from
Oct 25, 2017
Merged

Slim down npm package #1589

merged 1 commit into from
Oct 25, 2017

Conversation

avaly
Copy link
Contributor

@avaly avaly commented Oct 14, 2017

Purpose (TL;DR) - mandatory

Slim down NPM package in size

Background (Problem in detail) - optional

Sinon is downloaded from NPM around 3.8 millions times per month.

This change will save around 3.1 MB of uncompressed files and 0.9 MB of NPM package tarball (3.42 TB of NPM traffic per month).

Solution - optional

The build step should only create one set of packaged files.

The NPM CDN URL is still functional, since it supports package versioning: https://cdn.jsdelivr.net/npm/[email protected]/pkg/sinon.js

How to verify - mandatory

  1. Check out this branch
  2. npm install
  3. npm run build
  4. npm pack
  5. npm run postversion

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached.

@fatso83
Copy link
Contributor

fatso83 commented Oct 25, 2017

@mroderick Do you see any problems with this?

@mroderick
Copy link
Member

I haven’t had time to take a closer look due to vacation. I’ll look into it soon

@fatso83
Copy link
Contributor

fatso83 commented Oct 25, 2017

I don't see any immediate problems with it, as I would assume very few people are depending on the actual file format of the versioned files. And if they do, they probably refer to older versions, which means pruning the extra files going forwards should be fine. A 👍 from me.

@mroderick
Copy link
Member

I took a closer look, this looks great!

Thank you for the PR!

@mroderick mroderick merged commit a23f2d2 into sinonjs:master Oct 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants