Use babel in the build process. #900
Conversation
manthey
force-pushed
the
use-babel
branch
2 times, most recently
from
72413c0 to
c4a967d
Compare
|
This is probably a good point to introduce a general set of polyfills to continue to support the oldest browsers we want to handle. Specifically, |
There was a problem hiding this comment.
I am less familiar with babel but the changes looks reasonable. Perhaps @zachmullen can approve this branch?
|
There is something amiss with how coverage is being generated. I'm looking in to it. |
|
We use |
|
Using |
|
It turns out my issue with coverage was caused by using Babel 7 and, switching back to Babel 6, on where babel-polyfills was required. I've switched to Babel 6 and only added the polyfills to the non-lean build (and also require them in testing). |
Babel 7 supports only adding polyfills for features that are used, resulting in a smaller bundle than babel 6. By using karma-phantomjs-shim and @babel/polyfills, we no longer need our own polyfills module.
Babel 7 is still pending official release and causes some issues. Babel 7.0.0-rc.2 throws errors where 7.0.0-rc.1 does not. Rather than figure out what causes these, I recommend we wait until it is officially released. The main benefit of Babel 7 is that the polyfills can be added conditionally based on actual use. When adding babel-polyfill to src/index.js, the coverage for index.js is wrong (the line numbers are incorrect). Further, babel-polyfill is fairly heavy, so just add it to the vendor.js (geo.js not geo.lean.js). It also needs to be added to the test-utils to ensure that we can use new language features in the tests with PhantomJS. Reference to babel-polyfill has been moved to src/polyfills.js, and that file has been excluded from coverage analysis. For the coverage reporters, disable the html reporter as it fails with karma-coverage and babel. One solution would be to switch to karma-coverage-istanbul-reporter, but I don't think the html coverage is strictly necessary as we use codecov.
manthey
dismissed stale reviews from aashish24 and zachmullen
Fixed coverage and polyfill issues.
|
@zachmullen Can you review this again, please. |
karma-cov.conf.js
Outdated
| @@ -34,16 +34,17 @@ module.exports = function (config) { | |||
| karma_config.specReporter = {suppressPassed: true, suppressSkipped: true}; | |||
| karma_config.coverageReporter = { | |||
| reporters: [ | |||
| {type: 'html', dir: 'dist/coverage/', subdir: subdir_name}, | |||
| // {type: 'html', dir: 'dist/coverage/', subdir: subdir_name}, | |||
There was a problem hiding this comment.
Unless there's a good reason, we should probably remove unused lines rather than commenting them out.
This uses babel as a webpack loader to preprocess source files. It uses
babel-preset-envto decide what browsers to target, usingbrowsers: ['defaults']as the criterion. Perhaps we want to explicitly target a set of browsers.This resolves #418.