fix bundling; closes #3091 #3145

boneskull · 2017-12-12T10:39:35Z

This became too difficult to manage on the command-line.

What we're trying to do here is ship a version of the Buffer shim
which plays well with IE9/IE10. Browserify ships with a version which
does NOT play well, meaning we have to force it to use the version
we choose (buffer@4.9.x).

The fix is in two parts:

insertGlobalVars option replaces usages of global Buffer with
require('/path/to/mocha/node_modules/buffer').Buffer
Any other module which explicitly requires buffer or, yes,
buffer/, must also use /path/to/mocha/node_modules/buffer

If both of these are not in place, Browserify will use its own
version of the buffer shim.

boneskull · 2017-12-12T10:40:11Z

WIP pending some way of testing this so it doesn't happen again.

boneskull · 2017-12-12T10:44:14Z

oh also this is necessary because —require in browserify doesn’t just “alias” stuff like the “browser” field of package.json which doesn’t work for any modules outside of your own sources anyway...

boneskull · 2017-12-12T10:47:21Z

I started to wonder if this would be easier with Webpack then realized the build script I just wrote is likely smaller than even the most basic Webpack config 😬

coveralls · 2017-12-12T10:53:44Z

Coverage remained the same at 89.95% when pulling c05db7b on issue/3091 into 4c0df70 on master.

Bamieh · 2017-12-12T13:46:22Z

@boneskull I am afraid of using webpack in mocha, mainly because it overrides the native module implementation of node, which might lead to really odd bugs for some end users. It also limits some possible features of mocha, such as using node VMs for isolating the runner from the user code.

boneskull · 2017-12-12T20:55:42Z

@Bamieh We don't bundle for Node.js, only for the browser. So we don't need to worry about native modules getting blasted

Bamieh · 2017-12-12T21:10:30Z

@boneskull okay, i will test this at my work tomorrow because we have the appropriate testing environment for this issue.

This became too difficult to manage on the command-line. What we're trying to do here is ship a version of the `Buffer` shim which plays well with IE9/IE10. Browserify ships with a version which does NOT play well, meaning we have to force it to use the version we choose (`buffer@4.9.x`). The fix is in two parts: 1. `insertGlobalVars` option replaces usages of global `Buffer` with `require('/path/to/mocha/node_modules/buffer').Buffer` 2. Any *other* module which explicitly requires `buffer` or, yes, `buffer/`, must *also* use `/path/to/mocha/node_modules/buffer` If *both* of these are not in place, Browserify will use its *own* version of the `buffer` shim.

- this will assert that using `import` with `mocha.js` does not break - fix: remove missing `Makefile` target; add `test-browser-esm` target - fix: update invalid comments regarding running SauceLabs locally in `karma.conf` - fix: break if attempting to run Karma on AppVeyor - refactor Karma test flags to all use `MOCHA_TEST` env var - a few reformats Signed-off-by: Christopher Hiller <boneskull@boneskull.com>

boneskull · 2017-12-12T22:05:52Z

@Bamieh I've automated it.. please review

boneskull · 2017-12-12T22:08:16Z

Makefile

 KARMA := "node_modules/.bin/karma"
 MOCHA := "bin/mocha"
 NYC := "node_modules/.bin/nyc"

 ifdef COVERAGE
 define test_node
-	$(NYC) --no-clean --report-dir coverage/reports/$(1) $(MOCHA)
+$(NYC) --no-clean --report-dir coverage/reports/$(1) $(MOCHA)


from what I can tell the leading tab here is unnecessary

boneskull · 2017-12-12T22:08:26Z

Makefile

 endef
 else
-	test_node := $(MOCHA)
+test_node := $(MOCHA)


boneskull · 2017-12-12T22:08:43Z

karma.conf.js

@@ -4,6 +4,9 @@ var fs = require('fs');
 var path = require('path');
 var mkdirp = require('mkdirp');
 var baseBundleDirpath = path.join(__dirname, '.karma');
+var builder = require('./scripts/build');


this allows us to reuse the config in karma

boneskull · 2017-12-12T22:08:59Z

karma.conf.js

          .on('bundled', function (err, content) {
-            if (!err && bundleDirpath) {
+            if (err) {
+              throw err;


apparently we were eating errors before... oops.

boneskull · 2017-12-12T22:09:14Z

karma.conf.js

              // write bundle to directory for debugging
-              fs.writeFileSync(path.join(bundleDirpath,
-                'bundle.' + Date.now() + '.js'), content);
+              fs.writeFileSync(path.join(bundleDirpath, 'mocha.' + Date.now() +


I changed this filename

boneskull · 2017-12-12T22:09:47Z

karma.conf.js

+      if (cfg.sauceLabs) {
+        cfg.sauceLabs.testName = 'ESM Integration Tests';
+        cfg.browsers = ['chrome@latest'];
+        var launcher = cfg.customLaunchers['chrome@latest'];


sorry this is awkward; we need to update the dev scripts to ES2015+.

boneskull · 2017-12-12T22:10:05Z

karma.conf.js

+          'chrome@latest': launcher
+        };
+      } else if (!env.TRAVIS) {
+        cfg.browsers = ['Chrome'];


what other browsers currently support ES modules? @mochajs/core

in accordance to MDN

This feature is only just beginning to be implemented in browsers natively at this time. It is implemented in many transpilers, such as TypeScript and Babel, and bundlers such as Rollup and Webpack.

boneskull · 2017-12-12T22:10:29Z

karma.conf.js

+        process.exit(0);
+      }
+      cfg.files = [
+        'test/browser-fixtures/esm.fixture.html',


can we force Karma to load a .js file as a module instead of script? I have no idea.

boneskull · 2017-12-12T22:11:50Z

test/browser-fixtures/esm.fixture.html

@@ -0,0 +1,7 @@
+<script>
+  delete window.require;


this ensures that breakage will occur if the suffix on the distfile is bad.

see #3139 for what I mean

coveralls · 2017-12-12T22:23:42Z

Coverage remained the same at 89.968% when pulling add3281 on issue/3091 into 575d83d on master.

boneskull · 2017-12-13T03:40:55Z

You know, it might be a better idea to just land #3148 instead.

Bamieh · 2017-12-13T10:09:50Z

@boneskull i am totally in favor of #3148 instead of these hacks. But we should keep the tests you wrote if we agree on dropping support for IE9, IE10.

boneskull · 2018-01-02T19:23:46Z

need to pull my tests out of here

boneskull added the semver-patch implementation requires increase of "patch" version number; "bug fixes" label Dec 12, 2017

boneskull requested review from markowsiak, Bamieh and ScottFreeCode December 12, 2017 10:39

This was referenced Dec 12, 2017

Cannot find module issue with mocha.js in browser #3091

Closed

Can not import Mocha as a native ES6 module in a browser #3139

Closed

boneskull added 2 commits December 12, 2017 14:04

boneskull force-pushed the issue/3091 branch from c05db7b to add3281 Compare December 12, 2017 22:04

boneskull commented Dec 12, 2017

View reviewed changes

Makefile

endef

else

test_node := $(MOCHA)

test_node := $(MOCHA)

Copy link

Member Author

boneskull Dec 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

boneskull commented Dec 12, 2017

View reviewed changes

boneskull closed this Dec 31, 2017

boneskull deleted the issue/3091 branch January 9, 2018 04:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix bundling; closes #3091 #3145

fix bundling; closes #3091 #3145

boneskull commented Dec 12, 2017 •

edited

boneskull commented Dec 12, 2017

boneskull commented Dec 12, 2017

boneskull commented Dec 12, 2017

coveralls commented Dec 12, 2017

Bamieh commented Dec 12, 2017 •

edited

boneskull commented Dec 12, 2017

Bamieh commented Dec 12, 2017

boneskull commented Dec 12, 2017

boneskull Dec 12, 2017 •

edited

boneskull Dec 12, 2017

boneskull Dec 12, 2017

boneskull Dec 12, 2017

boneskull Dec 12, 2017

boneskull Dec 12, 2017

boneskull Dec 12, 2017 •

edited

Bamieh Dec 13, 2017

boneskull Dec 12, 2017

boneskull Dec 12, 2017

boneskull Dec 12, 2017

coveralls commented Dec 12, 2017

boneskull commented Dec 13, 2017

Bamieh commented Dec 13, 2017

boneskull commented Jan 2, 2018

fix bundling; closes #3091 #3145

fix bundling; closes #3091 #3145

Conversation

boneskull commented Dec 12, 2017 • edited

boneskull commented Dec 12, 2017

boneskull commented Dec 12, 2017

boneskull commented Dec 12, 2017

coveralls commented Dec 12, 2017

Bamieh commented Dec 12, 2017 • edited

boneskull commented Dec 12, 2017

Bamieh commented Dec 12, 2017

boneskull commented Dec 12, 2017

boneskull Dec 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boneskull Dec 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 12, 2017

boneskull commented Dec 13, 2017

Bamieh commented Dec 13, 2017

boneskull commented Jan 2, 2018

boneskull commented Dec 12, 2017 •

edited

Bamieh commented Dec 12, 2017 •

edited

boneskull Dec 12, 2017 •

edited

boneskull Dec 12, 2017 •

edited