Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Remove coverage files from npm package #1668

Merged
merged 1 commit into from

4 participants

@redonkulus

Removing the *-coverage.js files from the build reduced the package size from 33mb to 17mb. I'm unaware of whether the coverage files are used in production for YUI loaders filter option or just for code coverage purposes. If the latter, given tests are already removed, seems that coverage files should be removed as well.

For the results, I ran npm pack with and without the changes. Then expanded the files and ran du to check the size:

// before the change
orig.tgz is 5.2mb
$ tar -xf orig.tgz
$ du -h -d 1 orig
 34M    orig

// after removing coverage files
minus-coverage.tgz 2.7mb
$ tar -xf minus-coverage.tgz
$ du -h -d 1 minus-coverage
 17M    minus-coverage
@ericf
Owner

:thumbsup: Makes sense to me since we don't even include the src/ dir in the npm package; no reason to include the coverage files.

@caridy
Owner

last time we discussed this (probably more than a year ago), we discussed how these files can be useful not only for coverage, but also to compute the % of usage of the library, saying, how can you know how much do you really need from YUI, and whether you should use a sugar/shim method from YUI vs a low level implementation or a micro library. although, I haven't done that, I don't know anyone who does that, and if someone really want to do that, they can probably build the library and do it manually, or maybe releasing just a different pkg in npm called yui-with-coverage or something like that.

@redonkulus

@ericf @caridy given this module only is used on the server, does it make sense to remove minified files too?

@caridy
Owner

@redonkulus no, just like the client side, you can require different versions of the library on the server side. If you look into express-yui for example, you will see that we do:

YUI = require('yui' + (utils.debugMode ? '/debug' : ''));

which means we will load the debug version when node is running in debug mode or the raw version, we don't use the min version because for the server side, raw does the same thing.

@redonkulus

@caridy so if we do not use min, doesn't that mean we can remove it?

@tivac

I don't see why you'd need to ship either -coverage or -min in the npm version, & I'd love for those distributions to be as small as possible.

Unrelated to this issue, but it'd be super-cool if parts of the library could be broken out into their own small packages ala lodash. I often only want Template.Micro or Loader, for instance.

@caridy
Owner

ok ok, let's hold the horses :)

many people use the npm to actually serve static files for their apps (we do it with express-yui too when on development), that means all filters should just work. removing min is not possible at the moment, and this is even more important once we finally remove the build folder from github.

@caridy
Owner

@tivac about the micro-libraries, that's the direction we are moving, slowly, but we are getting there. @juandopazo just released ypromise, we released a bunch of stuff for intl (official announcement coming soon), and we are activity working on loader at the moment. we are also learning how to best integrate those pieces to maintain backward compatibility, and I'm sure ypromise will be a good exercise.

@ericf
Owner

I vote:
+ -min.js
- -converage.js

@redonkulus

Removing min files too gets us to 15mb.

@tivac

@caridy Out of curiosity I started writing a little package generator for specific YUI modules that uses loader to resolve dependencies & will eventually spit out NPM-compatible module bundles. The last part is still missing, as I haven't got package.json copying written up yet & I may not be copying quite everything I need just yet.

Still, I think the idea has promise and the code is stupidly-simple.

https://github.com/tivac/yui-clumper

@redonkulus

@caridy @ericf what are the next steps for this?

@ericf
Owner

@redonkulus I'm good with this PR how it stands now (removing -coverage.js files from the npm package.) I will merge this EOD, unless someone objects.

@ericf ericf merged commit 26a9c5a into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 28, 2014
  1. @redonkulus
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 0 deletions.
  1. +1 −0  .npmignore
View
1  .npmignore
@@ -2,3 +2,4 @@
src
sandbox
!build
+*-coverage.js
Something went wrong with that request. Please try again.