Build process + random updates

[marinko-peso]

• add eslint and lint existing source
• add bili (rolloup wrapper) to compile code and make it available in umd and commonjs
• add gitingore and npmignore
• create dist builds on publish and test

Reason for this is mostly bili: currently this package is not working properly with webpack prod build, uglifyJS is failing on it and it needs to get compiled by babel by adding an exception for node_modules. This PR resolves that problem and makes it plug & play even in prod environment, while still working properly in commonJS environment.

I hope it gets accepted! :-)

P.S. Tests are passing with the compiled version (and the src)

[mathiasvr]

Hi @marinko-peso,
I think adding webpack support is a good idea 👍

I would prefer to keep the repo small, so I suggest only adding the changes related to bili.
This means that the only new file will be bili.config.js (package-lock is also okay).
For linting, we use standard style (no semicolons), as only dependency.

Also, I'm not familiar with bili, is there any benefits by not using rollup in this case?

Let me know if you think there is a problem with this,
but if you can update the PR with these changes I am happy to merge it.

[marinko-peso]

Hey @mathiasvr , I was already starting to worry you are not gonno answer :-)

I respect your intent regarding the repo size and the lint style so I removed all the additional changes except gitignore which I need to prevent node_modules from being included. Its a standard js gitignore and won't do any harm.
Bili is just rolloup made super easy and it basically calls rollup in the background (built by egoist and that man know his stuff). It will auto generate es and um versions on prepublish and it will get auto picked up by node/webpack depending on the use case.
Btw I am using your package and I like it, just this webpack problem where I have to add exception for it (500+ packages and only this one needs an exception) is annoying so lets solve it :-)

Thanks!

P.S. I ran tests after update and all is passing

[mathiasvr]

Thanks for doing all this, but I don't think we need gitignore and the dist folder.
You should be able to ignore node_modules in the parent project right?

I tried the umd build, but it does not seem to work in the browser because of the node EventEmitter,
is this also why it doesn't work for you, or is it because you need a es5 version?

Anyway, I will take a look at it later, and replace the EventEmitter to make it easier to support the browser.

[marinko-peso]

Hey @mathiasvr , so I deleted dist folder but keep in mind it will get auto created once build is compiled. Its npm standard to keep versions of library in a dist directory.
I don't have a parent project over this but I did .gitignore the .gitignore, not sure why you are not keeping it there but I will comply :-)

Seems you are not really catching whats failing here. If anyone tries to use this with webpack/babel configuration (so in browser technically) it will work up until they want to generate a production build. At that point in time UglifyJS is called which fails on the fact your library does not have an ES5 version and it can't understand what to do with it. Example of exception:
Unexpected token: name (Timer) [./node_modules/imports-loader?define=>false!./node_modules/tiny-timer/index.js:17,0]

In order to resolve this user would have to understand the problem and put a special exception just for this lib to also be processed by Babel, for example:
exclude: /node_modules/(?!tiny-timer)/

When you publish a new version of the library with bili generated build webpack will automatically pull es5 version, like it does for all other libs out there, and UglifyJS will understand what to do with it. In the same time, when used in commonJS environment, commonJS module will be picked automatically.

I encourage you to try it with webpack prod build and see the problem yourself.

[mathiasvr]

I try to only use .gitignore for things that are specific to the project. In this case a .gitignore that ignores the dist folder would actually make sense, but anyway, I use a global .gitignore for all library/framework/IDE related files.

I tried webpack 4.12 with production flags, and I don't get any errors, but I don't know your exact setup.

If an es5 build solves the problem, I think we should go with that for now.
I have merged and released as 1.2.0.

However, I have updated bili to produce a commonjs build instead, because the current umd build does not work directly in the browser.

I hope this work for you, but since webpack uses the es build, it should not be a problem.

[marinko-peso]

Webpack 4.x has UglifyJS that supports ES6 so thats why you don't get an error. Most of the community is still on Webpack 3.x where UglifyJS doesn't work with ES6, and then it blows up.

It does solve the problem, umd build would be the standard but this also works. ES build generated by bili is es5, so when webpack puts it in the bundle before uglifyJS its fully compatible. In short, now it runs with no exception on any version of webpack :-)

UMD should work in the browser though, I'll double check that.
Thanks again for putting up with me, after all I think we made this library a tad better 👍

[mathiasvr]

@marinko-peso No problem, after all you were the one putting up with me. 😄

When reading up on this, I realized that many uses umd, but i guess i'm just hard to please.
IMO umd is not a good way to support multiple environments.
Libraries should expect the main entry to point to a node module (commonjs), and then webpack etc. can transform that for the browser.

The reason umd does not currently work natively in the browser (without webpack), is because the library depends on the node events module. This can be fixed by using node-builtins plugin for rollup, but that inlines a lot of extra shim in the umd that is completely unnecessary for node, webpack etc.

As I see it, the best way is to separate the build into commonjs for most libraries, es for the newest, and an inlined iife for the browser.

The only good argument I see for umd is to support amd, but is there honestly anyone who uses that anymore? And even in that case, I would only replace the iife with umd, and never refer it in package.json.

Sorry for ranting, I did not expect this comment to be get this long, just my usual frustration over javascript tooling. ¯\_(ツ)_/¯

Anyway, I'm happy with how it works now 👍