Skip to content

Commit

Permalink
Build: Update to Rollup 2 and Babel 7
Browse files Browse the repository at this point in the history
=== Problem and motivation

This fixes a runtime exception in IE 11.

> 'Symbol' is undefined
> dist/qunit.js:5505:11

The original code is:

```
for ( const hiddenTest of hiddenTests ) {
	tests.removeChild( hiddenTest );
}
```

which was previously compiled to:

```
for (var _iterator = hiddenTests[Symbol.iterator](), …
```

While it's fair and well-known that using ES6 APIs (e.g. Map,
Promise etc.) requires polyfills and isn't part of transpiling,
in this case the use of Symbol isn't in the original source,
which makes it kind of surprising. This is reported upstream
at babel/babel#11263 and was fixed in
Babel 7.

This led to a cascading chain of other build tooling packages
needing updating which brought in a larger chain of breaking
changes, renamed or removed options.

=== Babel and es6-promise

The main observable change here is that we no longer import the
ES6 version of the es6-promise polyfill (which we then compiled
with Babel locally), but instead import the ready-for-use
file provided by upstream, which seems to be what Babel and
Rollup recommend nowadays.

I've reviewed the dist/ file by hand (ignoring whitespace changes),
and also confirmed there are only superficial changes. Generally
still much the same as before. I've also tested it in IE11 and
it now generally works again, apart from a handful of failing
tests that I'll fix in a separate commit.

=== Rollup

To keep the rollup config simpler and to allow all upstream
Rollup documentation to apply directly, I've removed the
grunt-rollup indirection and call it rollup directly instead.

This also brings in a ton of useful debug information and output
about what is happening which seems to be surpressed by default
in grunt-rollup. I'm sure there's a way to make it appear, but
I didn't know that this useful output information existed.

== Find reporter

Rollup 2.x makes use of the "exports" declaration in its package.json
file, and it does not include package.json itself in the list of
declared exports, which means `require('rollup/package.json')`
results in an error that is different from MODULE_NOT_FOUND.

Update src/cli/find-reporter.js to accomodate this.
  • Loading branch information
Krinkle committed Aug 16, 2020
1 parent 6859235 commit 484d396
Show file tree
Hide file tree
Showing 7 changed files with 1,623 additions and 875 deletions.
12 changes: 7 additions & 5 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{
"presets": [
// Don't transpile modules since Rollup does that
[ "env", { modules: false } ]
],
"plugins": [ "external-helpers" ]
"presets": [
[ "@babel/preset-env", {
targets: {
ie: 9
}
} ]
]
}
10 changes: 1 addition & 9 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,6 @@ module.exports = function( grunt ) {
dest: "dist/qunit.js"
}
},
rollup: {
options: require( "./rollup.config" ),
src: {
src: "src/qunit.js",
dest: "dist/qunit.js"
}
},
eslint: {
options: {
config: ".eslintrc.json"
Expand Down Expand Up @@ -292,12 +285,11 @@ module.exports = function( grunt ) {
} );

grunt.loadTasks( "build/tasks" );
grunt.registerTask( "build", [ "rollup:src", "copy:src-js", "copy:src-css" ] );
grunt.registerTask( "build-copy", [ "copy:src-js", "copy:src-css" ] );
grunt.registerTask( "test-base", [ "eslint", "search", "test-on-node" ] );
grunt.registerTask( "test", [ "test-base", "connect:nolivereload", "qunit" ] );
grunt.registerTask( "test-in-watch", [ "test-base", "qunit" ] );
grunt.registerTask( "coverage", [
"build",
"instrument",
"copy:dist-to-tmp",
"copy:instrumented-to-dist",
Expand Down
Loading

0 comments on commit 484d396

Please sign in to comment.