Skip to content

Commit

Permalink
Disable Babel modules transform for dependencies (#455)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
goto-bus-stop authored Apr 13, 2018
1 parent a4306c5 commit cee3689
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
24 changes: 16 additions & 8 deletions lib/graph-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 })

Expand Down
28 changes: 28 additions & 0 deletions test/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit cee3689

Please sign in to comment.