Rollup and babel in build pipeline #4293

Munter · 2020-05-17T10:39:50Z

We want to modernize the code base, and in order to do so we need to add a transpilation step for building the browser artifacts we distribute, so we can retain our current conservative browser version targets.

The strategy I suggest is applying Babel in our bundling tool to keep the output browser compatible. I suggest switching from browserify to Babel to give us an ability to move to ES modules when we want to, but alsy get the immediate option of creating multiple distributions with different configurations of transpilation applied.

When these changes are all done we should be able to convert the code base to more modern js, allowing things like destructuring, arrow functions etc.

Description of the Change

Clean up code paths, so node-only code isn't part of a browser bundles dependency graph
Replace Browserify with Rollup
~~Set up eslint to keep the code node compatible at a version we decide~~
Add Babel transpilation to rollup bundles
- Configure browser output that is IE11 compatible --> mocha.js
- ~~Configure a more modern output for evergreen browsers --> mocha.modern.js?~~

Alternate Designs

We could have retained browserify as our bundling tool, but rollup was chosen for its ability to deal with ES modules, which is a very likely future for Mocha, given its browser nature.

Benefits

No more restrictions on authors to write ES5 and remember what browser globals were available 7 years ago

Possible Drawbacks

Changing bundling strategy can carry a risk, especially if the bundler changes the way it resolves node global for a browser.

Applying transpilation can carry a risk of not knowing the exact output that will be generated.

We need to test these changes very well.

coveralls · 2020-05-17T10:50:18Z

Coverage increased (+0.2%) to 93.932% when pulling 7739689 on munter/transpilation into e30ae28 on master.

boneskull · 2020-05-20T22:50:23Z

needs https://npm.im/process

boneskull · 2020-05-27T18:45:44Z

rebased onto master

Munter · 2020-06-13T07:07:25Z

This branch is ready to be merged IMO. All the transpilation config has been added, which makes the code base ready to be converted to modern JS.

I made an example branch where I ran lebab -t arrow on our lib files: https://github.com/mochajs/mocha/compare/munter/transpilation...munter/lebab?expand=1
The travis build for that is green, meaning that it runs in IE11, where arrow functions would normally throw: https://travis-ci.org/github/mochajs/mocha

I think we should merge this branch separately from doing the actual code modernisation, as an attempt to keep the change-set as small as possible here and make it easier to coordinate when we pull in modernisations that will likely require other branches to resolve conflicts

Munter · 2020-06-13T07:08:12Z

The failing Netlify deploy is something about spriting opencollective images, which also happens on other branches. I'll look into that somewhere else

juergba · 2020-06-15T08:06:37Z

The strategy I suggest is applying Babel in our bundling tool to keep the output browser compatible.

This means our code plus the code of our dependencies will be transpiled, right?

Munter · 2020-06-15T09:01:34Z

This means our code plus the code of our dependencies will be transpiled, right?

I'm pretty sure the current config will also transpile node_modules, but I haven't actually tested it. Maybe we should make a new branch based on this one and upgrade a dependency we know will break IE11. The Travis build should tell us if it works

boneskull

great work. pls see my comments etc. mostly questions, but a couple things I'd like changed (none of them are huge)

how do we get away with removing lib/browser/tty.js?

are there any breaking changes here?

.gitignore

lib/reporters/base.js

package-scripts.js

scripts/karma-rollup-plugin.js

lib/cli/lookupFiles.js

package.json

rollup.config.js

test/unit/parseQuery.spec.js

juergba

@Munter from a bird's-eye view this PR makes sense and looks good to me. I can't evaluate the details though. We should take the risk and merge it (after addressing @boneskull's comments).

.browserslistrc

scripts/karma-rollup-plugin.js

juergba · 2020-06-19T08:13:23Z

Maybe we should make a new branch based on this one and upgrade a dependency we know will break IE11.

@Munter I tried to make this test. But I can't get your branch munter/transpilation (unchanged) running on my local Windows machine. npm test fails at script test.browser.unit with code ELIFECYCLE. The bundling of ./mocha.js succeeds with a warning of circular dependencies, but above script fails.

Edit: I cleaned the npm cache, deleted the node_modules folder and package-lock.json file. Then npm install. The browser tests fail. I don't know wether this is a local problem on my laptop only, since we don't run any browser tests on Appveyor.

Edit2: By adding failOnEmptyTestSuite: false to karma.conf.js, npm test passes, but no browser tests are run. The summary always states: SUMMARY: √ 0 tests completed.
So the root problem is, the karma-runner does not execute any tests on Windows.

package.json

karma.conf.js

Munter · 2020-06-20T12:54:14Z

adding failOnEmptyTestSuite: false to karma.conf.js, npm test passes, but no browser tests are run. The summary always states: SUMMARY: √ 0 tests completed.
So the root problem is, the karma-runner does not execute any tests on Windows.

I'd say this is a blocker. You should be able to develop locally on windows.

I'm a bit confused why our tests from saucelabs are not representative of your experience here.

const browserPlatformPairs = {
  'chrome@latest': 'Windows 10',
  'MicrosoftEdge@latest': 'Windows 10',
  'internet explorer@latest': 'Windows 10',
  'firefox@latest': 'Windows 10',
  'safari@latest': 'macOS 10.13'
};

I guess it could be something with the bundling mechanism, since the saucelabs tests are run on linux bundled assets I think.

Maybe we need appveyor to run an additional test of karma and a local headless chrome

juergba · 2020-06-20T14:07:22Z

My local output shows:

nps is executing `test.browser.unit` : cross-env NODE_PATH=. karma start --single-run
CI mode disabled

START:
babelHelpers: 'bundled' option was used by default. It is recommended to configure this option explicitly, read more here: 
https://github.com/rollup/plugins/tree/master/packages/babel#babelhelpers
Generated an empty chunk: "_rollup:plugin-multi-entry:entry-point"
Generated an empty chunk: "_rollup:plugin-multi-entry:entry-point"
20 06 2020 16:00:19.035:INFO [karma-server]: Karma v4.4.1 server started at http://0.0.0.0:9876/
20 06 2020 16:00:19.046:INFO [launcher]: Launching browsers ChromeHeadless with concurrency unlimited
20 06 2020 16:00:19.070:INFO [launcher]: Starting browser ChromeHeadless
20 06 2020 16:00:22.356:INFO [HeadlessChrome 83.0.4103 (Windows 10.0.0)]: Connected on socket NylQbcaiMkoAHu2QAAAA with id 
56280182

Finished in 0.012 secs / 0 secs @ 16:00:22 GMT+0200 (GMT+02:00)

SUMMARY:
√ 0 tests completed

These two empty chunks look odd. In Travis' logfile you don't find such a warning.

juergba · 2020-06-20T14:35:48Z

What about this rollup-plugin-glob-import?
Doesn't it bundle multi test files (also glob's) into one single test file?

Munter · 2020-06-21T11:47:02Z

What about this rollup-plugin-glob-import?
Doesn't it bundle multi test files (also glob's) into one single test file?

@juergba That plugin works on globs in import statements in code. Useful if you want to bundle thing you can't statically analyse the need for. Like icons or dynamic content etc. We could possibly use it, but only if we created a new entry file for each of our test branches from Karma and import those globs. I'm unsure how that would work with a new wrapper layer i import/exports. It didn't seem very appealing

Munter · 2020-06-21T11:51:37Z

These two empty chunks look odd. In Travis' logfile you don't find such a warning

@juergba I think I might have messed something up that caused the different windows paths with backslashes not to match in the plugin. I'll be at the office tomorrow and will bring home a windows laptop so I have a setup I can recreate and fix it in.

I still think it would be a good idea to add an appveyor test for running headless chrome tests on windows

boneskull · 2020-06-29T22:16:30Z

I'll give this a try on windows too

boneskull · 2020-06-30T19:09:14Z

I'm running this side-by-side on mac and windows, and the result of nps build is different. windows builds one thing, and mac builds something else.

boneskull · 2020-06-30T19:12:30Z

alright, the difference is just some object prop ordering.

but I can definitely reproduce the "can't run browser tests on windows" problem :(

boneskull · 2020-06-30T19:14:57Z

it creates an empty bundle. we see warnings

Generated an empty chunk: "_rollup:plugin-multi-entry:entry-point"
Generated an empty chunk: "_rollup:plugin-multi-entry:entry-point"

the contents of the bundle:

(function () {
	'use strict';



}());
//# sourceMappingURL=f16fa37b-ce80-4765-8342-10dd7125ff93.rollup.js.map

boneskull · 2020-06-30T20:40:44Z

OK, 008da88 should fix it. @juergba can you give it a shot?

boneskull · 2020-06-30T20:43:56Z

I'm unsure what the root cause is here. perhaps there's something in karma that demands POSIX paths?

Signed-off-by: Christopher Hiller <[email protected]>

boneskull · 2020-07-01T21:24:24Z

(someone needs to rebase this manually before merging)

boneskull · 2020-07-01T22:30:51Z

I marked this patch but I'm not entirely confident it won't break somebody.

@mantoni I wanted to let you know that this exists and will be merged soon. I am not sure if this is going to be good or bad for you, or neither. 😄

mantoni · 2020-07-02T07:38:25Z

@boneskull Thank you for thinking of me. My current experiments with a rewrite of Mochify depend on the pre-bundled version of Mocha. So this won't be an issue.

…liasing

Munter · 2020-07-02T14:10:41Z

I'd like to see if I can silence a few of the rollup warnings before we merge

…on and aliasing" This reverts commit 207ef63.

Munter marked this pull request as draft May 17, 2020 10:39

boneskull force-pushed the munter/transpilation branch from b7e7517 to 72583f0 Compare May 27, 2020 18:45

boneskull mentioned this pull request May 28, 2020

Support --require of ESM; closes #4281 #4304

Merged

Munter force-pushed the munter/transpilation branch from 72583f0 to ca71ddc Compare June 12, 2020 18:58

Munter marked this pull request as ready for review June 13, 2020 07:02

Munter requested review from boneskull and a team June 13, 2020 07:08

boneskull suggested changes Jun 15, 2020

View reviewed changes

juergba approved these changes Jun 17, 2020

View reviewed changes

.browserslistrc Show resolved Hide resolved

scripts/karma-rollup-plugin.js Outdated Show resolved Hide resolved

scripts/karma-rollup-plugin.js Outdated Show resolved Hide resolved

juergba reviewed Jun 19, 2020

View reviewed changes

package.json Show resolved Hide resolved

juergba reviewed Jun 20, 2020

View reviewed changes

karma.conf.js Show resolved Hide resolved

Munter and others added 9 commits July 1, 2020 12:39

Add browserslist, babel, babel-preset-env and prepare for modern js

7fcd39a

Add mocha.js.map to npm distributed files

eedd4e6

Use util function to check if we're running in a browser

822515a

Improve documentation of self-built karma-rollup-plugin

052acb6

Renamed lib/cli/lookupFiles.js to lib/cli/lookup-files.js

eaf7220

Clean up rollup config

1a6176c

Kebab case file names

ac4aa46

Remove last traces of unused browserify

b8c30ed

fix bundle path under win32

5b08dec

Signed-off-by: Christopher Hiller <[email protected]>

boneskull force-pushed the munter/transpilation branch from bf3662e to 5b08dec Compare July 1, 2020 21:40

do not run bundle visualizer in CI

ea59f88

boneskull approved these changes Jul 1, 2020

View reviewed changes

boneskull added type: chore generally involving deps, tooling, configuration, etc. area: repository tooling concerning ease of contribution semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Jul 1, 2020

boneskull mentioned this pull request Jul 1, 2020

implementation of global setup/teardown; closes #4308 #4360

Merged

Munter added 3 commits July 2, 2020 14:02

Remove use of mocha.opts in browser testing

6be6606

Replace package.json browser field with manual rollup exclusion and a…

207ef63

…liasing

Improved naming of rollup plugin to pick values from package.json

f01aeba

Revert "Replace package.json browser field with manual rollup exclusi…

7739689

…on and aliasing" This reverts commit 207ef63.

boneskull merged commit 40f951b into master Jul 2, 2020

Munter deleted the munter/transpilation branch July 2, 2020 21:47

boneskull mentioned this pull request Jul 28, 2020

build UMD bundle and polyfill language features #4366

Merged

boneskull added this to the next milestone Jul 29, 2020

m90 mentioned this pull request Jul 23, 2021

window._webdriver_manualPoll is not a function on BrowserStack since 8.0.0 mantoni/mochify.js#238

Open

Omrisnyk mentioned this pull request Dec 7, 2023

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#131

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollup and babel in build pipeline #4293

Rollup and babel in build pipeline #4293

Munter commented May 17, 2020 •

edited by boneskull

Loading

coveralls commented May 17, 2020 •

edited

Loading

boneskull commented May 20, 2020

boneskull commented May 27, 2020

Munter commented Jun 13, 2020

Munter commented Jun 13, 2020

juergba commented Jun 15, 2020

Munter commented Jun 15, 2020

boneskull left a comment

juergba left a comment

juergba commented Jun 19, 2020 •

edited

Loading

Munter commented Jun 20, 2020

juergba commented Jun 20, 2020

juergba commented Jun 20, 2020

Munter commented Jun 21, 2020

Munter commented Jun 21, 2020

boneskull commented Jun 29, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jul 1, 2020

boneskull commented Jul 1, 2020

mantoni commented Jul 2, 2020

Munter commented Jul 2, 2020

Rollup and babel in build pipeline #4293

Rollup and babel in build pipeline #4293

Conversation

Munter commented May 17, 2020 • edited by boneskull Loading

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

coveralls commented May 17, 2020 • edited Loading

boneskull commented May 20, 2020

boneskull commented May 27, 2020

Munter commented Jun 13, 2020

Munter commented Jun 13, 2020

juergba commented Jun 15, 2020

Munter commented Jun 15, 2020

boneskull left a comment

Choose a reason for hiding this comment

juergba left a comment

Choose a reason for hiding this comment

juergba commented Jun 19, 2020 • edited Loading

Munter commented Jun 20, 2020

juergba commented Jun 20, 2020

juergba commented Jun 20, 2020

Munter commented Jun 21, 2020

Munter commented Jun 21, 2020

boneskull commented Jun 29, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jun 30, 2020

boneskull commented Jul 1, 2020

boneskull commented Jul 1, 2020

mantoni commented Jul 2, 2020

Munter commented Jul 2, 2020

Munter commented May 17, 2020 •

edited by boneskull

Loading

coveralls commented May 17, 2020 •

edited

Loading

juergba commented Jun 19, 2020 •

edited

Loading