diff --git a/CHANGELOG.md b/CHANGELOG.md index 39a6c754bfb..4375938ba5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ The relative path fix in the previous release caused a regression where paths in source maps contained `\` instead of `/` on Windows. That is incorrect because source map paths are URLs, not file system paths. This release replaces `\` with `/` for consistency on Windows. +* `module.require()` is now an alias for `require()` ([#455](https://github.com/evanw/esbuild/issues/455)) + + Some packages such as [apollo-server](https://github.com/apollographql/apollo-server) use `module.require()` instead of `require()` with the intent of bypassing the bundler's `require` and calling the underlying function from `node` instead. Unfortunately that doesn't actually work because CommonJS module semantics means `module` is a variable local to that file's CommonJS closure instead of the host's `module` object. + + This wasn't an issue when using `apollo-server` with Webpack because the literal expression `module.require()` is automatically rewritten to `require()` by Webpack: [webpack/webpack#7750](https://github.com/webpack/webpack/pull/7750). To get this package to work, esbuild now matches Webpack's behavior here. Calls to `module.require()` will become external calls to `require()` as long as the required path has been marked as external. + ## 0.7.15 * Lower `export * as` syntax for ES2019 and below diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 73a59c36918..35da90eb485 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -7562,7 +7562,7 @@ func (p *parser) jsxStringsToMemberExpression(loc logger.Loc, parts []string, as if i+1 == len(parts) { targetIfLast = assignTarget } - if expr, ok := p.maybeRewritePropertyAccess(loc, targetIfLast, js_ast.OptionalChainNone, value, parts[i], loc); ok { + if expr, ok := p.maybeRewritePropertyAccess(loc, targetIfLast, js_ast.OptionalChainNone, value, parts[i], loc, false); ok { value = expr } else { value = js_ast.Expr{Loc: loc, Data: &js_ast.EDot{ @@ -7662,6 +7662,7 @@ func (p *parser) maybeRewritePropertyAccess( target js_ast.Expr, name string, nameLoc logger.Loc, + isCallTarget bool, ) (js_ast.Expr, bool) { if id, ok := target.Data.(*js_ast.EIdentifier); ok { // Rewrite property accesses on explicit namespace imports as an identifier. @@ -7710,6 +7711,14 @@ func (p *parser) maybeRewritePropertyAccess( p.recordUsage(item.Ref) return p.handleIdentifier(nameLoc, assignTarget, &js_ast.EIdentifier{Ref: item.Ref}), true } + + // Rewrite "module.require()" to "require()" for Webpack compatibility. + // See https://github.com/webpack/webpack/pull/7750 for more info. + if isCallTarget && id.Ref == p.moduleRef && name == "require" { + p.ignoreUsage(p.moduleRef) + p.recordUsage(p.requireRef) + return js_ast.Expr{Loc: nameLoc, Data: &js_ast.EIdentifier{Ref: p.requireRef}}, true + } } // If this is a known enum value, inline the value of the enum @@ -8597,7 +8606,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO out = exprOut{childContainsOptionalChain: containsOptionalChain} if str, ok := e.Index.Data.(*js_ast.EString); ok { name := js_lexer.UTF16ToString(str.Value) - if value, ok := p.maybeRewritePropertyAccess(expr.Loc, in.assignTarget, e.OptionalChain, e.Target, name, e.Index.Loc); ok { + if value, ok := p.maybeRewritePropertyAccess(expr.Loc, in.assignTarget, e.OptionalChain, e.Target, name, e.Index.Loc, isCallTarget); ok { return value, out } } @@ -8794,7 +8803,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO // Potentially rewrite this property access out = exprOut{childContainsOptionalChain: containsOptionalChain} - if value, ok := p.maybeRewritePropertyAccess(expr.Loc, in.assignTarget, e.OptionalChain, e.Target, e.Name, e.NameLoc); ok { + if value, ok := p.maybeRewritePropertyAccess(expr.Loc, in.assignTarget, e.OptionalChain, e.Target, e.Name, e.NameLoc, isCallTarget); ok { return value, out } return js_ast.Expr{Loc: expr.Loc, Data: e}, out diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 434ddd324da..19bf55acd6d 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -111,6 +111,57 @@ }), ) + // Test CommonJS semantics + tests.push( + // "module.require" should work with internal modules + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {foo, req} from './foo'`, + 'foo.js': `exports.req = module.require; exports.foo = module.require('./bar')`, + 'bar.js': `exports.bar = 123`, + 'node.js': `if (require('./out').foo.bar !== 123 || require('./out').req !== undefined) throw 'fail'`, + }), + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {foo, req} from './foo'`, + 'foo.js': `exports.req = module['require']; exports.foo = module['require']('./bar')`, + 'bar.js': `exports.bar = 123`, + 'node.js': `if (require('./out').foo.bar !== 123 || require('./out').req !== undefined) throw 'fail'`, + }), + + // "module.require" should work with external modules + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs', '--external:fs'], { + 'in.js': `export {foo} from './foo'`, + 'foo.js': `exports.foo = module.require('fs').exists`, + 'node.js': `if (require('./out').foo !== require('fs').exists) throw 'fail'`, + }), + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {foo} from './foo'`, + 'foo.js': `let fn = (m, p) => m.require(p); exports.foo = fn(module, 'fs').exists`, + 'node.js': `try { require('./out') } catch (e) { return } throw 'fail'`, + }), + + // "module.exports" should behave like a normal property + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {foo} from './foo'`, + 'foo.js': `exports.foo = module.exports`, + 'node.js': `if (require('./out').foo !== require('./out').foo.foo) throw 'fail'`, + }), + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {default} from './foo'`, + 'foo.js': `module.exports = 123`, + 'node.js': `if (require('./out').default !== 123) throw 'fail'`, + }), + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {default} from './foo'`, + 'foo.js': `let m = module; m.exports = 123`, + 'node.js': `if (require('./out').default !== 123) throw 'fail'`, + }), + test(['--bundle', 'in.js', '--outfile=out.js', '--format=cjs'], { + 'in.js': `export {default} from './foo'`, + 'foo.js': `let fn = (m, x) => m.exports = x; fn(module, 123)`, + 'node.js': `if (require('./out').default !== 123) throw 'fail'`, + }), + ) + // Test internal CommonJS export tests.push( test(['--bundle', 'in.js', '--outfile=node.js'], {