From ddc10590335e57bd6e6060190f51b6a04d716a35 Mon Sep 17 00:00:00 2001 From: Chad Killingsworth Date: Thu, 1 Dec 2016 13:58:15 -0800 Subject: [PATCH] Remove IIFE wrappers from CommonJS modules that have UMD patterns Closes https://github.com/google/closure-compiler/pull/2146 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=140774013 --- .../jscomp/FunctionToBlockMutator.java | 64 ++++- .../jscomp/ProcessCommonJSModules.java | 100 ++++++- .../jscomp/ProcessCommonJSModulesTest.java | 272 ++++++++++++++++-- 3 files changed, 397 insertions(+), 39 deletions(-) diff --git a/src/com/google/javascript/jscomp/FunctionToBlockMutator.java b/src/com/google/javascript/jscomp/FunctionToBlockMutator.java index 090d0fddae3..b2d77b8cb1f 100644 --- a/src/com/google/javascript/jscomp/FunctionToBlockMutator.java +++ b/src/com/google/javascript/jscomp/FunctionToBlockMutator.java @@ -63,16 +63,62 @@ class FunctionToBlockMutator { */ Node mutate(String fnName, Node fnNode, Node callNode, String resultName, boolean needsDefaultResult, boolean isCallInLoop) { + return mutateInternal( + fnName, fnNode, callNode, resultName, needsDefaultResult, isCallInLoop, + /* renameLocals */ true); + } + + /** + * Used when an IIFE wrapper is being removed + * + * @param fnName The name to use when preparing human readable names. + * @param fnNode The function to prepare. + * @param callNode The call node that will be replaced. + * @return A clone of the function body mutated to be suitable for injection as a statement into a + * script root + */ + Node unwrapIifeInModule(String fnName, Node fnNode, Node callNode) { + return mutateInternal(fnName, fnNode, callNode, + /* resultName */ null, + /* needsDefaultResult */ false, + /* isCallInLoop */ false, + /* renameLocals */ false); + } + + /** + * @param fnName The name to use when preparing human readable names. + * @param fnNode The function to prepare. + * @param callNode The call node that will be replaced. + * @param resultName Function results should be assigned to this name. + * @param needsDefaultResult Whether the result value must be set. + * @param isCallInLoop Whether the function body must be prepared to be injected into the body of + * a loop. + * @param renameLocals If the inlining is part of module rewriting and doesn't require making + * local names unique + * @return A clone of the function body mutated to be suitable for injection as a statement into + * another code block. + */ + private Node mutateInternal( + String fnName, + Node fnNode, + Node callNode, + String resultName, + boolean needsDefaultResult, + boolean isCallInLoop, + boolean renameLocals) { Node newFnNode = fnNode.cloneTree(); - // Now that parameter names have been replaced, make sure all the local - // names are unique, to allow functions to be inlined multiple times - // without causing conflicts. - makeLocalNamesUnique(newFnNode, isCallInLoop); - - // Function declarations must be rewritten as function expressions as - // they will be within a block and normalization prevents function - // declarations within block as browser implementations vary. - rewriteFunctionDeclarations(newFnNode.getLastChild()); + + if (renameLocals) { + // Now that parameter names have been replaced, make sure all the local + // names are unique, to allow functions to be inlined multiple times + // without causing conflicts. + makeLocalNamesUnique(newFnNode, isCallInLoop); + + // Function declarations must be rewritten as function expressions as + // they will be within a block and normalization prevents function + // declarations within block as browser implementations vary. + rewriteFunctionDeclarations(newFnNode.getLastChild()); + } // TODO(johnlenz): Mark NAME nodes constant for parameters that are not // modified. diff --git a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java index 331db00b922..604d18fae92 100644 --- a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java +++ b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java @@ -101,7 +101,17 @@ public void process(Node externs, Node root) { ImmutableList.Builder exports = ImmutableList.builder(); if (finder.isCommonJsModule()) { finder.reportModuleErrors(); - finder.replaceUmdPatterns(); + + if (!finder.umdPatterns.isEmpty()) { + finder.replaceUmdPatterns(); + + // Removing the IIFE rewrites vars. We need to re-traverse + // to get the new references. + if (removeIIFEWrapper(root)) { + finder = new FindImportsAndExports(); + NodeTraversal.traverseEs6(compiler, root, finder); + } + } //UMD pattern replacement can leave detached export references - don't include those for (ExportInfo export : finder.getModuleExports()) { @@ -156,6 +166,72 @@ private Node getBaseQualifiedNameNode(Node n) { return refParent; } + /** + * UMD modules are often wrapped in an IIFE for cases where they are used as scripts instead of + * modules. Remove the wrapper. + * @return Whether an IIFE wrapper was found and removed. + */ + private boolean removeIIFEWrapper(Node root) { + Preconditions.checkState(root.isScript()); + Node n = root.getFirstChild(); + + // Sometimes scripts start with a semicolon for easy concatenation. + // Skip any empty statements from those + while (n != null && n.isEmpty()) { + n = n.getNext(); + } + + // An IIFE wrapper must be the only non-empty statement in the script, + // and it must be an expression statement. + if (n == null || !n.isExprResult() || n.getNext() != null) { + return false; + } + + Node call = n.getFirstChild(); + if (call == null || !call.isCall()) { + return false; + } + + // Find the IIFE call and function nodes + Node fnc; + if (call.getFirstChild().isFunction()) { + fnc = n.getFirstFirstChild(); + } else if (call.getFirstChild().isGetProp() + && call.getFirstFirstChild().isFunction() + && call.getFirstFirstChild().getNext().isString() + && call.getFirstFirstChild().getNext().getString().equals("call")) { + fnc = call.getFirstFirstChild(); + + // We only support explicitly binding "this" to the parent "this" + if (!(call.getSecondChild() != null && call.getSecondChild().isThis())) { + return false; + } + } else { + return false; + } + + if (NodeUtil.isVarArgsFunction(fnc)) { + return false; + } + + CompilerInput ci = compiler.getInput(root.getInputId()); + ModulePath modulePath = ci.getPath(); + if (modulePath == null) { + return false; + } + + String iifeLabel = modulePath.toModuleName() + "_iifeWrapper"; + + FunctionToBlockMutator mutator = + new FunctionToBlockMutator(compiler, compiler.getUniqueNameIdSupplier()); + Node block = mutator.unwrapIifeInModule(iifeLabel, fnc, call); + root.removeChildren(); + root.addChildrenToFront(block.removeChildren()); + compiler.reportCodeChange(); + + return true; + } + /** * Traverse the script. Find all references to CommonJS require (import) and module.exports or * export statements. Add goog.require statements for any require statements. Rewrites any require @@ -503,6 +579,11 @@ void replaceUmdPatterns() { } p.replaceChild(umdPattern.ifRoot, newNode); } + + + if (!umdPatterns.isEmpty()) { + compiler.reportCodeChange(); + } } } @@ -658,11 +739,23 @@ private void visitExport(NodeTraversal t, Node export) { } } + ModulePath modulePath = t.getInput().getPath(); + String moduleName = modulePath.toModuleName(); + // If this is an assignment to module.exports or exports, renaming // has already handled this case. Remove the export. Var rValueVar = null; if (rValue != null && rValue.isQualifiedName()) { rValueVar = t.getScope().getVar(rValue.getQualifiedName()); + + // If the exported name is not found and this is a direct assignment + // to modules.exports, look to see if the module name has a var definition + if (rValueVar == null && root == export) { + rValueVar = t.getScope().getVar(moduleName); + if (rValueVar != null && rValueVar.getNode() == root) { + rValueVar = null; + } + } } if (root.getParent().isAssign() @@ -674,21 +767,20 @@ private void visitExport(NodeTraversal t, Node export) { return; } - ModulePath modulePath = t.getInput().getPath(); - String moduleName = modulePath.toModuleName(); Node updatedExport = NodeUtil.newName(compiler, moduleName, export, export.getQualifiedName()); - // Ensure that direct assignments to "module.exports" have var definitions if ("module.exports".equals(root.getQualifiedName()) && rValue != null && t.getScope().getVar("module.exports") == null && root.getParent().isAssign() && root.getParent().getParent().isExprResult()) { + // Rewrite "module.exports = foo;" to "var moduleName = foo;" Node parent = root.getParent(); Node var = IR.var(updatedExport, rValue.detach()).useSourceInfoFrom(root.getParent()); parent.getParent().replaceWith(var); } else { + // Other references to "module.exports" are just replaced with the module name. export.replaceWith(updatedExport); } compiler.reportCodeChange(); diff --git a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java index 3e12bb3138b..db6945349b4 100644 --- a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java +++ b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java @@ -57,7 +57,9 @@ public void testWithoutExports() { testModules( "var name = require('./other'); name()", LINE_JOINER.join( - "goog.require('module$other');", "var name = module$other;", "module$other();")); + "goog.require('module$other');", + "var name = module$other;", + "module$other();")); test( ImmutableList.of( SourceFile.fromCode(Compiler.joinPathParts("mod", "name.js"), ""), @@ -110,7 +112,9 @@ public void testExportsInExpression() { setFilename("test"); testModules( LINE_JOINER.join( - "var name = require('./other');", "var e;", "e = module.exports = function() {};"), + "var name = require('./other');", + "var e;", + "e = module.exports = function() {};"), LINE_JOINER.join( "goog.provide('module$test');", "goog.require('module$other');", @@ -121,7 +125,8 @@ public void testExportsInExpression() { testModules( LINE_JOINER.join( - "var name = require('./other');", "var e = module.exports = function() {};"), + "var name = require('./other');", + "var e = module.exports = function() {};"), LINE_JOINER.join( "goog.provide('module$test');", "goog.require('module$other');", @@ -130,7 +135,9 @@ public void testExportsInExpression() { "var e$$module$test = module$test = function () {};")); testModules( - LINE_JOINER.join("var name = require('./other');", "(module.exports = function() {})();"), + LINE_JOINER.join( + "var name = require('./other');", + "(module.exports = function() {})();"), LINE_JOINER.join( "goog.provide('module$test');", "goog.require('module$other');", @@ -143,7 +150,9 @@ public void testPropertyExports() { setFilename("test"); testModules( LINE_JOINER.join( - "exports.one = 1;", "module.exports.obj = {};", "module.exports.obj.two = 2;"), + "exports.one = 1;", + "module.exports.obj = {};", + "module.exports.obj.two = 2;"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test = {};", @@ -160,7 +169,9 @@ public void testPropertyExports() { public void testModuleExportsWrittenWithExportsRefs() { setFilename("test"); testModules( - LINE_JOINER.join("exports.one = 1;", "module.exports = {};"), + LINE_JOINER.join( + "exports.one = 1;", + "module.exports = {};"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test={};", @@ -171,7 +182,9 @@ public void testVarRenaming() { setFilename("test"); testModules( LINE_JOINER.join( - "module.exports = {};", "var a = 1, b = 2;", "(function() { var a; b = 4})();"), + "module.exports = {};", + "var a = 1, b = 2;", + "(function() { var a; b = 4})();"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test={};", @@ -182,7 +195,9 @@ public void testVarRenaming() { public void testDash() { setFilename("test-test"); testModules( - LINE_JOINER.join("var name = require('./other');", "exports.foo = 1;"), + LINE_JOINER.join( + "var name = require('./other');", + "exports.foo = 1;"), LINE_JOINER.join( "goog.provide('module$test_test');", "goog.require('module$other');", @@ -194,7 +209,9 @@ public void testDash() { public void testIndex() { setFilename("foo/index"); testModules( - LINE_JOINER.join("var name = require('../other');", "exports.bar = 1;"), + LINE_JOINER.join( + "var name = require('../other');", + "exports.bar = 1;"), LINE_JOINER.join( "goog.provide('module$foo$index');", "goog.require('module$other');", @@ -206,7 +223,9 @@ public void testIndex() { public void testModuleName() { setFilename("foo/bar"); testModules( - LINE_JOINER.join("var name = require('../other');", "module.exports = name;"), + LINE_JOINER.join( + "var name = require('../other');", + "module.exports = name;"), LINE_JOINER.join( "goog.provide('module$foo$bar');", "goog.require('module$other');", @@ -291,7 +310,9 @@ public void testUMDPatternConversion() { "} else {", " this.foobar = foobar;", "}"), - LINE_JOINER.join("goog.provide('module$test');", "var module$test = {foo: 'bar'};")); + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};")); testModules( LINE_JOINER.join( @@ -303,7 +324,9 @@ public void testUMDPatternConversion() { "} else {", " this.foobar = foobar;", "}"), - LINE_JOINER.join("goog.provide('module$test');", "var module$test = {foo: 'bar'};")); + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};")); testModules( LINE_JOINER.join( @@ -314,7 +337,9 @@ public void testUMDPatternConversion() { "if (typeof define === 'function' && define.amd) {", " define([], function () {return foobar;});", "}"), - LINE_JOINER.join("goog.provide('module$test');", "var module$test = {foo: 'bar'};")); + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};")); } public void testEs6ObjectShorthand() { @@ -323,7 +348,11 @@ public void testEs6ObjectShorthand() { setFilename("test"); testModules( LINE_JOINER.join( - "function foo() {}", "module.exports = {", " prop: 'value',", " foo", "};"), + "function foo() {}", + "module.exports = {", + " prop: 'value',", + " foo", + "};"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test = {};", @@ -347,7 +376,9 @@ public void testEs6ObjectShorthand() { "};")); testModules( - LINE_JOINER.join("var a = require('./other');", "module.exports = {a: a};"), + LINE_JOINER.join( + "var a = require('./other');", + "module.exports = {a: a};"), LINE_JOINER.join( "goog.provide('module$test');", "goog.require('module$other');", @@ -356,7 +387,9 @@ public void testEs6ObjectShorthand() { "module$test.a = module$other;")); testModules( - LINE_JOINER.join("var a = require('./other');", "module.exports = {a};"), + LINE_JOINER.join( + "var a = require('./other');", + "module.exports = {a};"), LINE_JOINER.join( "goog.provide('module$test');", "goog.require('module$other');", @@ -365,7 +398,9 @@ public void testEs6ObjectShorthand() { "module$test.a = module$other;")); testModules( - LINE_JOINER.join("var a = 4;", "module.exports = {a};"), + LINE_JOINER.join( + "var a = 4;", + "module.exports = {a};"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test = {};", @@ -397,7 +432,9 @@ public void testFunctionRewriting() { setFilename("test"); testModules( LINE_JOINER.join( - "function foo() {}", "foo.prototype = new Date();", "module.exports = foo;"), + "function foo() {}", + "foo.prototype = new Date();", + "module.exports = foo;"), LINE_JOINER.join( "goog.provide('module$test');", "var module$test = function() {};", @@ -405,7 +442,9 @@ public void testFunctionRewriting() { testModules( LINE_JOINER.join( - "function foo() {}", "foo.prototype = new Date();", "module.exports = {foo: foo};"), + "function foo() {}", + "foo.prototype = new Date();", + "module.exports = {foo: foo};"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test = {};", @@ -417,7 +456,9 @@ public void testFunctionHoisting() { setFilename("test"); testModules( LINE_JOINER.join( - "module.exports = foo;", "function foo() {}", "foo.prototype = new Date();"), + "module.exports = foo;", + "function foo() {}", + "foo.prototype = new Date();"), LINE_JOINER.join( "goog.provide('module$test');", "var module$test = function() {};", @@ -428,16 +469,25 @@ public void testClassRewriting() { setLanguage(CompilerOptions.LanguageMode.ECMASCRIPT6, CompilerOptions.LanguageMode.ECMASCRIPT5); setFilename("test"); testModules( - LINE_JOINER.join("class foo extends Array {}", "module.exports = foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "let module$test = class extends Array {}")); + "class foo extends Array {}", + "module.exports = foo;"), + LINE_JOINER.join( + "goog.provide('module$test');", + "let module$test = class extends Array {}")); testModules( - LINE_JOINER.join("class foo {}", "module.exports = foo;"), - LINE_JOINER.join("goog.provide('module$test');", "let module$test = class {}")); + LINE_JOINER.join( + "class foo {}", + "module.exports = foo;"), + LINE_JOINER.join( + "goog.provide('module$test');", + "let module$test = class {}")); testModules( - LINE_JOINER.join("class foo {}", "module.exports.foo = foo;"), + LINE_JOINER.join( + "class foo {}", + "module.exports.foo = foo;"), LINE_JOINER.join( "goog.provide('module$test');", "/** @const */ var module$test = {};", @@ -480,7 +530,9 @@ public void testDestructuringImports() { setLanguage(CompilerOptions.LanguageMode.ECMASCRIPT6, CompilerOptions.LanguageMode.ECMASCRIPT5); setFilename("test"); testModules( - LINE_JOINER.join("const {foo, bar} = require('./other');", "var baz = foo + bar;"), + LINE_JOINER.join( + "const {foo, bar} = require('./other');", + "var baz = foo + bar;"), LINE_JOINER.join( "goog.require('module$other');", "const {foo, bar} = module$other;", @@ -502,6 +554,174 @@ public void testAnnotationsCopied() { "/** @type {string} */ module$test.a.prototype.foo;")); } + public void testUMDRemoveIIFE() { + setFilename("test"); + testModules( + LINE_JOINER.join( + "(function(){", + "var foobar = {foo: 'bar'};", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " this.foobar = foobar;", + "}})()"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};")); + + testModules( + LINE_JOINER.join( + ";;;(function(){", + "var foobar = {foo: 'bar'};", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " this.foobar = foobar;", + "}})()"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};")); + + testModules( + LINE_JOINER.join( + "(function(){", + "var foobar = {foo: 'bar'};", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " this.foobar = foobar;", + "}}.call(this))"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};")); + + testModules( + LINE_JOINER.join( + ";;;(function(global){", + "var foobar = {foo: 'bar'};", + "global.foobar = foobar;", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " global.foobar = foobar;", + "}})(this)"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};", + "this.foobar = module$test;")); + + testModules( + LINE_JOINER.join( + "(function(global){", + "var foobar = {foo: 'bar'};", + "global.foobar = foobar;", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " global.foobar = foobar;", + "}}.call(this, this))"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {foo: 'bar'};", + "this.foobar = module$test;")); + + // We can't remove IIFEs explict calls that don't use "this" + testModules( + LINE_JOINER.join( + "(function(){", + "var foobar = {foo: 'bar'};", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " this.foobar = foobar;", + "}}.call(window))"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {};", + "(function(){", + " module$test={foo:\"bar\"};", + "}).call(window);")); + + // Can't remove IIFEs when there are sibling statements + testModules( + LINE_JOINER.join( + "(function(){", + "var foobar = {foo: 'bar'};", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " this.foobar = foobar;", + "}})();", + "alert('foo');"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {};", + "(function(){", + " module$test={foo:\"bar\"};", + "})();", + "alert('foo');")); + + // Can't remove IIFEs when there are sibling statements + testModules( + LINE_JOINER.join( + "alert('foo');", + "(function(){", + "var foobar = {foo: 'bar'};", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " this.foobar = foobar;", + "}})();"), + LINE_JOINER.join( + "goog.provide('module$test');", + "var module$test = {};", + "alert('foo');", + "(function(){", + " module$test={foo:\"bar\"};", + "})();")); + + // Annotations for local names should be preserved + testModules( + LINE_JOINER.join( + "(function(global){", + "/** @param {...*} var_args */", + "function log(var_args) {}", + "var foobar = {foo: 'bar', log: function() { log.apply(null, arguments); } };", + "global.foobar = foobar;", + "if (typeof module === 'object' && module.exports) {", + " module.exports = foobar;", + "} else if (typeof define === 'function' && define.amd) {", + " define([], function() {return foobar;});", + "} else {", + " global.foobar = foobar;", + "}}.call(this, this))"), + LINE_JOINER.join( + "goog.provide('module$test');", + "/** @param {...*} var_args */", + "function log$$module$test(var_args){}", + "var module$test = {", + " foo: 'bar',", + " log: function() { log$$module$test.apply(null,arguments); }", + "};", + "this.foobar = module$test;")); + } + public void testParamShadow() { setFilename("test"); testModules(