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

Installing moment independently is fine, but not moment-timezone depending on moment #1193

Closed
blixt opened this issue Oct 9, 2015 · 4 comments

Comments

@blixt
Copy link

blixt commented Oct 9, 2015

There seems to be a bug with how jspm pulls in dependencies (or at least a difficult to understand configuration issue) that I've run into with the moment-timezone package.

jspm install npm:moment works well, and jspm install npm:moment-timezone used to work well. But when moment-timezone added an explicit dependency to moment in version 0.4.1, jspm started pulling in lots of extra dependencies weighing in at over 150 kB. All the dependencies appear to be shims that enable a complete Node.js environment in the browser (buffers, streams, process, etc etc), but these are not things that moment depends on so I don't understand why it's happening.

You can reproduce the issue with these steps in an empty directory:

$ npm init  # just use all the defaults
$ npm install jspm --save-dev
$ jspm init  # just use all the defaults
$ echo "import moment from 'moment-timezone'; console.log(moment().format());" > index.js
# First, let's try v0.4.0
$ jspm install npm:moment-timezone@0.4.0
$ jspm bundle-sfx index index.bundled.js
$ ls -la
# Now, let's try v0.4.1
$ jspm install npm:moment-timezone@0.4.1
$ jspm bundle-sfx index index.bundled.js
$ ls -la

When following the above steps, I see the bundle size going from 105 kB (0.4.0) to 255 kB (0.4.1).

Just to confirm it's not actually the moment package, do this in another empty directory:

$ npm init  # just use all the defaults
$ npm install jspm --save-dev
$ jspm init  # just use all the defaults
$ echo "import moment from 'moment'; console.log(moment().format());" > index.js
$ jspm install npm:moment@~2.6.0 # the dependency specified in moment-timezone@0.4.0
$ jspm bundle-sfx index index.bundled.js
$ ls -la

The bundle is 73 kB here, which doesn't account for the 150 kB bump I was seeing above, even if we don't discount all the bootstrapping code.

@guybedford
Copy link
Member

@blixt the commits in moment-timezone are:

Of those, the first three would make no difference to jspm

So the problems are moment/moment-timezone@ad09aa0 and moment/moment-timezone@9bf892e causing issues here.

The first one would likely be the cause of unnecessary dependencies being installed.

The second one may well explain the bundle size increase.

@blixt
Copy link
Author

blixt commented Oct 10, 2015

Thanks for looking into this. The first one, how does it cause additional dependencies to be installed? Does jspm inspect all the .js files in the builds directory and finds reason to include additional dependencies? Wouldn't they be cleaned out during the bundle step if they're not used? I also tried with --minify which makes the bundled size 43 kB for v0.4.0 and 208 kB for v0.4.1.

By the way, here's the diff of the v0.4.0 and v0.4.1 trees (using du -d2 -h jspm_packages format):

2.8M  jspm_packages/github/jmcriffey                2.8M  jspm_packages/github/jmcriffey
 48K  jspm_packages/github/jspm                 |   212K  jspm_packages/github/jspm
 32K  jspm_packages/github/systemjs                  32K  jspm_packages/github/systemjs
2.9M  jspm_packages/github                      |   3.0M  jspm_packages/github
 48K  jspm_packages/npm/assert@1.3.0                 48K  jspm_packages/npm/assert@1.3.0
                                                >    32K  jspm_packages/npm/base64-js@0.0.8
                                                >    40K  jspm_packages/npm/Base64@0.2.1
                                                >   196K  jspm_packages/npm/browserify-zlib@0.1.4
                                                >   208K  jspm_packages/npm/buffer@3.5.1
                                                >    36K  jspm_packages/npm/core-util-is@1.0.1
                                                >   108K  jspm_packages/npm/events@1.0.2
                                                >    20K  jspm_packages/npm/https-browserify@0.0.0
                                                >    32K  jspm_packages/npm/ieee754@1.1.6
 24K  jspm_packages/npm/inherits@2.0.1               24K  jspm_packages/npm/inherits@2.0.1
2.3M  jspm_packages/npm/moment-timezone@0.4.0   |    36K  jspm_packages/npm/is-array@1.0.1
484K  jspm_packages/npm/moment@2.10.6           |    24K  jspm_packages/npm/isarray@0.0.1
                                                >   2.7M  jspm_packages/npm/moment-timezone@0.4.1
                                                >   384K  jspm_packages/npm/moment@2.6.0
                                                >   780K  jspm_packages/npm/pako@0.2.8
                                                >    24K  jspm_packages/npm/path-browserify@0.0.0
 28K  jspm_packages/npm/process@0.11.2               28K  jspm_packages/npm/process@0.11.2
                                                >    24K  jspm_packages/npm/punycode@1.3.2
                                                >    80K  jspm_packages/npm/querystring@0.2.0
                                                >   116K  jspm_packages/npm/readable-stream@1.1.13
                                                >    28K  jspm_packages/npm/stream-browserify@1.0.0
                                                >    28K  jspm_packages/npm/string_decoder@0.10.31
                                                >    92K  jspm_packages/npm/url@0.10.3
 84K  jspm_packages/npm/util@0.10.3                  84K  jspm_packages/npm/util@0.10.3
3.0M  jspm_packages/npm                         |   5.2M  jspm_packages/npm
6.4M  jspm_packages                             |   8.9M  jspm_packages

@blixt
Copy link
Author

blixt commented Oct 10, 2015

I've been doing side by side comparison of the --minify'ed bundles and it does seem as if the files in builds are included. It's probably not jspm's fault so you can close this unless you think there's something fixable outside of the moment-timezone project.

@guybedford
Copy link
Member

jspm will parse all JS files, and ensure that all require statements that involve Node core modules have a browserify shim installed. So including new js files will include a greater surface area of Browserify shims.

The other thing to diff would be the list of modules shown when doing jspm bundle to see the actual modules system, instead of the file system.

Generally we track package configuration issues at https://github.com/jspm/registry/issues, feel free to continue this discussion there.

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

No branches or pull requests

2 participants