Replace Browserify and Babel with TypeScript and Rollup

[dstaley]

This is the first stage of the TypeScript RFC, where we replace Browserify and Babel with TypeScript and Rollup.

This PR mainly focuses on modifications to package.json to support building with TypeScript and Rollup. The new build process is as follows:

1. npm run build creates two versions of the library using the TypeScript compiler: an ES modules version written in ES2017, and a CommonJS modules version written in ES5.
2. npm run dist uses Rollup to convert the ES modules version of the library into two UMD outputs: kinto-http.min.js which is an ES5 version of the library, and temp.js which is an ES2017 version of the library.
3. temp.js is then appended to fx-src/jsm_prefix.js, and stored in dist/moz-kinto-http-client.js.

The testing workflow has been updated to use the original TypeScript source through the use of ts-node. The actual commands and workflow is unchanged. The same goes for coverage generation, which is now using Istanbul's CLI nyc.

ESLint has been updated to properly understand TypeScript source files.

package.json has been updated to reference the ES modules version under the module property, which means that developers using a modern bundler will get the ES modules/ES2017 version. The Babel polyfill has been removed.

I had to use a few Rollup plugins to account for the fact that uuid is a CommonJS dependency. Hopefully when their ES6 transition is complete we'll be able to remove rollup-plugin-node-resolve and rollup-plugin-commonjs.

[leplatrem]

I looked in the generated files in lib/cjs-es5/ and at least index.fx.js does not look like it contains the imported modules (eg. from ./base or from ./errors)
You can see more or less what is expected: https://searchfox.org/mozilla-central/rev/f91bd38732d4a330eba4e780812274b98eb81274/services/common/kinto-http-client.js#343
Once that sorted out, if you need me to test the output in my locally compiled Firefox let me know ;)
Thanks!

[dstaley]

@leplatrem The Gecko/Firefox build is in dist/moz-kinto-http-client.js. I've checked mine and it definitely contains both ./base and ./errors.

The lib/cjs-es5 folder is used in Node environments, and the index.fx.js file in that folder isn't actually used since Node will import beginning with index.js. The index.fx.js file is only there because that folder is the output of running src through the TypeScript compiler. If you think it would confuse people to see that file in the folder, we can remove it during the build step. But for the most part the only people that should ever see it would be those that spelunk into their node_modules folder.

[leplatrem]

I don't think it works on my setup. If I delete dist/* and run npm run build the moz-kinto-http-client.js file does not get created ;)

[leplatrem]

I don't think it works on my setup. If I delete dist/* and run npm run build the moz-kinto-http-client.js file does not get created ;)

npm run dist works, sorry 🤣

[leplatrem]

The tests pass in Gecko with the changes of #434 (unrelated)

[leplatrem]

I'm good with it.

I saw that you removed the noshim output. What is the rationale? Should we only keep the es5 bundle? Or do we keep the es6 bundle only?

[dstaley]

The generated bundle in dist is still ES5 (only the Gecko build is ES6). Removing the polyfill is mainly to reduce size since the ES5 build should support the vast majority of browsers. If someone wants to use it for an ancient browser it's easy for them to also include something like polyfill.io. I'm gonna merge in those ChromeUtils changes and then I think this is ready to merge from my end!

…

On Tue, Jul 2, 2019, 6:16 AM Mathieu Leplatre ***@***.***> wrote: ***@***.**** approved this pull request. I'm good with it. I saw that you removed the noshim output. What is the rationale? Should we only keep the es5 bundle? Or do we keep the es6 bundle only? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#433?email_source=notifications&email_token=AAAVQYZSOZVWZRZ67B6B4ULP5NIJPA5CNFSM4H4ALOR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5H36PQ#pullrequestreview-256884542>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAVQY2VVVUWZ4OC6ZPPNL3P5NIJPANCNFSM4H4ALORQ> .

[dstaley]

@leplatrem I've merged in those ChromeUtils changes. Rollup does this really neat thing where it will adjust the variable names to prevent shadowing a global. However, since this is the exact behavior we want, I added a small plugin to replace the aliased output from Rollup with the correct variable names.

Edit: I've updated this to use a smarter method instead of replacing strings. Now, Rollup will resolve all global uses of setTimeout and clearTimeout to the exports of fx-src/timer.js, which just exports those same functions from ChromeUtils. The generated build looks correct on my end, but would you mind giving this another test in Gecko to make sure the setTimeout and clearInterval functions from Timer.jsm are working correctly? (The import is now closer to the top of the bundle as opposed to the bottom.)

Also, is it at all feasible to add those Firefox tests to Travis? It'd be really nice to know that changes here don't break Firefox functionality!

[leplatrem]

I'll be off for the next 2 weeks :) @glasserc feel free to merge if it's all good on the Gecko side :) (with the last commit that removes inject)

[leplatrem]

The generated build looks correct on my end, but would you mind giving this another test in Gecko to make sure the setTimeout and clearInterval functions from Timer.jsm are working correctly?

I tested the output in Gecko and it works :)

Also, is it at all feasible to add those Firefox tests to Travis? It'd be really nice to know that changes here don't break Firefox functionality!

Unfortunately that's not an easy task. It's ok for us now to maintain our fork there and backport the changes here from time to time ;)

[dstaley]

@glasserc I just wanted to check in and see if there was anything I could help with to get this merged. I'd love to start working on adding types, but I'd rather get this merged first.

[leplatrem]

I'm back from holidays. Yes, I want to see this merged too. We had other priorities :(
Last time I check it was all good.

What do you think if you resolve the conflicts and just merge? If anything surprising comes up, I can still derive a branch from the last tag to avoid being blocked by this change.

[dstaley]

@leplatrem merged in those changes! Once you merge this in I'll go ahead and start working on adding the first set of types. Also, if you wouldn't mind, could you publish an alpha release to npm so I can start this process for the main repo as well?

[Natim]

Well done 🎉

[leplatrem]

I published an alpha version.