petkaantonov / bluebird

:bird: :zap: Bluebird is a full featured promise library with unmatched performance.
http://bluebirdjs.com
MIT License
20.44k stars 2.34k forks source link

Builds using esModules/Support Tree Shaking #1284

Open benderTheCrime opened 7 years ago

benderTheCrime commented 7 years ago

(This issue tracker is only for bug reports or feature requests, if this is neither, please choose appropriate channel from http://bluebirdjs.com/docs/support.html)

Please answer the questions the best you can:

1) What version of bluebird is the issue happening on? All of them.

2) What platform and version? (For example Node.js 0.12 or Google Chrome 32) Every Platform/Every Version

3) Did this issue happen with earlier version of bluebird? Yes

(Write description of your issue here, stack traces from errors and code that reproduces the issue are helpful)

I'm not sure if this is already represented in the Bluebird pipeline, or I've missed another issue that captures this (I just searched for "tree-shaking", entirely possible). If so, this can be closed.

It would be nice if Bluebird were bundled using a well-known module bundler, or could in some way capitalize on the tree-shaking functionality that many well-known module bundlers provide.

I am requiring Bluebird into a project of mine. It is quite large, but I am willing to include it because of the considerable speed advantages I get out of using it (seriously, awesome work). I am then using Webpack 2 (webpack@2.1.0-beta26) to bundle my project.

This project does not use any of the ancillary methods bluebird provides (map, filter, each, etc), but because the module is pre-bundled (using "fs"), I cannot shake any of the modules that are in the bluebird.js main.

In addition, I have tried explicitly including Bluebird in my build and referencing bluebird only from 'bluebird/js/release/bluebird':

loaders: [
    {
         test: /\.js$/,
         loader: 'babel-loader',
         exclude: /node_modules\/(?!bluebird)/
    }
]
import bluebird from 'bluebird/js/release/bluebird';

but this results in a build of exactly the same size. It appears as though a bluebird.core.js module is distributed as well, which is slightly smaller, but it also appears to not be as performant as the main module.

In addition, if there are any workarounds, I would love to try them out.

spion commented 7 years ago

Seems like the v3 documentation is missing this:

https://github.com/caitp/node-bluebird#custom-builds

But it should still work. For an up to date list, see https://github.com/petkaantonov/bluebird/blob/master/tools/build.js#L26

benderTheCrime commented 7 years ago

@spion I didn't know that was a possibility, thank you for passing along these links.

Nevertheless, I would classify this under the "workarounds" I mentioned in my issue. It is nice to be able to conditionalize the optional modules based on a build, but it seems like a lot of work for a dependency within any project's build lifecycle.

If I was to do this, I would have to explicitly add a statement to my build scripts that built bluebird without those modules every time my packages were resolved, and rebuild if my project starts to use one of the optional methods is called within my project (imposing some small amount of overhead).

Therefore, I think this is still a relevant issue to track. It's unrealistic to expect users to create implementation-specific builds when there is a perfectly good vehicle to prevent them from having to do so.

Perhaps https://github.com/rollup/rollup/wiki/jsnext:main?

benderTheCrime commented 7 years ago

Also, as the node-bluebird steps to create a custom build suggest, this has to be done from a cloned instance of bluebird. There is no gruntfile associated with Bluebird, so actually adding this to my build pipeline does not work.

spion commented 7 years ago

Right, the command has changed to ./build but other than that everything should work the same, e.g. node tools/build.js --features=core --no-debug --release --browser --minify

This builds core without "zalgo" (which should be removed anyway) which means it should act exactly the same as the full build, minus missing methods.

Bluebird's architecture is a bit... different, so I have doubts that rollup's automatic tree shaking will actually work on it. As far as I know, rollup cannot remove individual class methods ( which is what you would need to build a stripped-down version of bluebird). It can only deal with static ES6 imports and exports, which means it will either include the entire Promise class, or none of it.

Just fyi, the difference between the minified core build and the full build isn't that great:

118758 18 Nov 09:24 bluebird.core.js 55367 18 Nov 09:24 bluebird.core.min.js 178174 18 Nov 09:24 bluebird.js 79282 18 Nov 09:24 bluebird.min.js

Minified + gzipped its 22kb (full), 16kb (core)

benderTheCrime commented 7 years ago

I am not suggesting that Bluebird Rollup per se, just suggesting an exposed jsnext:main in the package.json that enabled the use of bluebird's modules as esModules.

I am curious about the reason Bluebird's architecture is so different. It's such a vastly popular library, I would think that there was an effort to make it uniformly consumable as modules.

spion commented 7 years ago

@benderTheCrime Its written as core + mixins that are applied to that core. One of the reasons why its like that is to enable custom builds. Otherwise, all features would be provided by a single class and you would not be able to configure what will be included at all (and rollup would not be able to eliminate anything)

petkaantonov commented 7 years ago

Also I am not sure if it is related but because every file is a callable function with dynamic mixins rather than static export, you can make multiple independent yet interoperating copies of the library this way. If a library wants to use bluebird and extend it somehow, they can make such copy and users will not get those extensions in their bluebird promise even if npm decides to use the same copy of the library.