From cee3689cb0c6e69ccfe631694b7a46b4ec9726c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= <renee@kooi.me>
Date: Fri, 13 Apr 2018 10:15:40 +0200
Subject: [PATCH] Disable Babel modules transform for dependencies (#455)

The Babel modules transform turns top level `this` into undefined. This
causes issues with some libraries that expect `this` to refer to the
global object (or at least expect it to exist). See #423 for an example.

Because browserify looks for the package.json `main` key, which should
not contain ES modules code, it should be safe to just disable the
modules transform.
---
 lib/graph-script.js | 24 ++++++++++++++++--------
 test/script.js      | 28 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/lib/graph-script.js b/lib/graph-script.js
index 290eadfd..064e6b14 100644
--- a/lib/graph-script.js
+++ b/lib/graph-script.js
@@ -46,12 +46,6 @@ function node (state, createEdge) {
   var browsers = browserslist(null, { path: entry })
   if (!browsers.length) browsers = defaultBrowsers
 
-  var babelPresets = [
-    [babelPresetEnv, {
-      targets: { browsers: browsers }
-    }]
-  ]
-
   if (state.metadata.watch) {
     b = watchify(b)
     debug('watching ' + entry)
@@ -64,20 +58,34 @@ function node (state, createEdge) {
   b.ignore('sheetify/insert')
   b.transform(sheetify)
   b.transform(glslify)
+
   if (state.metadata.babelifyDeps) {
     // Dependencies should be transformed, but their .babelrc should be ignored.
     b.transform(tfilter(babelify, { include: /node_modules/ }), {
       global: true,
       babelrc: false,
-      presets: babelPresets
+      presets: [
+        [babelPresetEnv, {
+          // browserify resolves the commonjs version of modules anyway,
+          // and the modules transform in babel-preset-env rewrites top level `this`
+          // to `undefined` which breaks some modules (underscore, cuid, ...)
+          modules: false,
+          targets: { browsers: browsers }
+        }]
+      ]
     })
   }
   // In our own code, .babelrc files should be used.
   b.transform(tfilter(babelify, { exclude: /node_modules/ }), {
     global: true,
     babelrc: true,
-    presets: babelPresets
+    presets: [
+      [babelPresetEnv, {
+        targets: { browsers: browsers }
+      }]
+    ]
   })
+
   b.transform(brfs, { global: true })
   b.transform(yoyoify, { global: true })
 
diff --git a/test/script.js b/test/script.js
index 788d7535..d491c001 100644
--- a/test/script.js
+++ b/test/script.js
@@ -4,6 +4,7 @@ var mkdirp = require('mkdirp')
 var path = require('path')
 var tape = require('tape')
 var fs = require('fs')
+var vm = require('vm')
 var os = require('os')
 var tmp = require('tmp')
 
@@ -194,6 +195,33 @@ tape('use custom browserslist config', function (assert) {
   })
 })
 
+tape('does not transform top level `this` in dependencies', function (assert) {
+  assert.plan(4)
+  var file = `
+    T.equal(require('a')(), 10)
+  `
+  var dependency = `
+    module.exports = this.number || (() => 10)
+  `
+
+  var tmpDirname = path.join(__dirname, '../tmp', 'js-pipeline-' + (Math.random() * 1e4).toFixed())
+  mkdirp.sync(path.join(tmpDirname, 'node_modules'))
+  fs.writeFileSync(path.join(tmpDirname, 'app.js'), file)
+  fs.writeFileSync(path.join(tmpDirname, 'node_modules', 'a.js'), dependency)
+
+  var compiler = bankai(path.join(tmpDirname, 'app.js'), { watch: false })
+  compiler.on('error', assert.error)
+  compiler.scripts('bundle.js', function (err, res) {
+    assert.error(err, 'no error writing script')
+    var content = res.buffer.toString('utf8')
+
+    assert.ok(/this\.number/.test(content), 'did not rewrite `this`')
+    assert.ok(/return 10/.test(content), 'did rewrite arrow function')
+
+    vm.runInNewContext(content, { T: assert })
+  })
+})
+
 tape('envify in watch mode', function (assert) {
   assert.plan(5)