diff --git a/src/com/google/javascript/jscomp/Es6RewriteModules.java b/src/com/google/javascript/jscomp/Es6RewriteModules.java index 9355dce6d54..e34067ad6ab 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteModules.java +++ b/src/com/google/javascript/jscomp/Es6RewriteModules.java @@ -103,36 +103,6 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback */ private Map importMap; - /** - * Local variable names that were goog.require'd to qualified name we need to line. We need to - * inline all required names since there are certain well-known Closure symbols (like - * goog.asserts) that later stages of the compiler check for and cannot handle aliases. - * - *

We use this to rewrite something like: - * - * 

-   *   import {x} from '';
-   *   const {assert} = goog.require('goog.asserts');
-   *   assert(x);
-   *
- * - * To: - * - * 
-   *   import {x} from '';
-   *   goog.asserts.assert(x);
-   *
- * - * Because if we used an alias like below the assertion would not be recognized: - * - * 
-   *   import {x} from '';
-   *   const {assert} = goog.asserts;
-   *   assert(x);
-   *
- */ - private Map namesToInlineByAlias; - private final Set importedGoogNames; private Set typedefs; @@ -209,7 +179,6 @@ public void clearState() { this.importMap = new HashMap<>(); this.importedGoogNames.clear(); this.typedefs = new HashSet<>(); - this.namesToInlineByAlias = new HashMap<>(); } /** @@ -617,26 +586,12 @@ private void rewriteRequires(Node script) { (NodeTraversal t, Node n, Node parent) -> { if (n.isCall()) { if (n.getFirstChild().matchesQualifiedName("goog.require")) { - visitRequireOrGet(t, n, parent, /* isRequire= */ true); + visitRequire(t, n, parent, true /* checkScope */); } else if (n.getFirstChild().matchesQualifiedName("goog.module.get")) { visitGoogModuleGet(t, n, parent); } } }); - NodeTraversal.traversePostOrder( - compiler, - script, - (NodeTraversal t, Node n, Node parent) -> { - if (n.isName() && namesToInlineByAlias.containsKey(n.getString())) { - Var v = t.getScope().getVar(n.getString()); - if (v == null || v.getNameNode() != n) { - Node replacement = - NodeUtil.newQName(compiler, namesToInlineByAlias.get(n.getString())); - replacement.useSourceInfoFromForTree(n); - n.replaceWith(replacement); - } - } - }); } private void visitGoogModuleGet(NodeTraversal t, Node getCall, Node parent) { @@ -651,18 +606,17 @@ private void visitGoogModuleGet(NodeTraversal t, Node getCall, Node parent) { return; } - visitRequireOrGet(t, getCall, parent, /* isRequire= */ false); + visitRequire(t, getCall, parent, false /* checkScope */); } - private void visitRequireOrGet( - NodeTraversal t, Node requireCall, Node parent, boolean isRequire) { + private void visitRequire(NodeTraversal t, Node requireCall, Node parent, boolean checkScope) { if (!requireCall.hasTwoChildren() || !requireCall.getLastChild().isString()) { t.report(requireCall, INVALID_REQUIRE_NAMESPACE); return; } // Module has already been turned into a script at this point. - if (isRequire && !t.getScope().isGlobal()) { + if (checkScope && !t.getScope().isGlobal()) { t.report(requireCall, INVALID_CLOSURE_CALL_ERROR); return; } @@ -683,31 +637,9 @@ private void visitRequireOrGet( } if (isStoredInDeclaration) { - if (isRequire) { - if (parent.isDestructuringLhs()) { - checkState(parent.getFirstChild().isObjectPattern()); - for (Node child : parent.getFirstChild().children()) { - if (child.isStringKey()) { - checkState(child.getFirstChild().isName()); - namesToInlineByAlias.put( - child.getFirstChild().getString(), - m.getGlobalName(namespace) + "." + child.getString()); - } else { - checkState(child.isName()); - namesToInlineByAlias.put( - child.getString(), m.getGlobalName(namespace) + "." + child.getString()); - } - } - } else { - checkState(parent.isName()); - namesToInlineByAlias.put(parent.getString(), m.getGlobalName(namespace)); - } - parent.getParent().detach(); - } else { - Node replacement = - NodeUtil.newQName(compiler, m.getGlobalName(namespace)).srcrefTree(requireCall); - parent.replaceChild(requireCall, replacement); - } + Node replacement = + NodeUtil.newQName(compiler, m.getGlobalName(namespace)).srcrefTree(requireCall); + parent.replaceChild(requireCall, replacement); } else { checkState(requireCall.getParent().isExprResult()); requireCall.getParent().detach(); diff --git a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java index 96f9983c187..45292e1baee 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java +++ b/test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java @@ -117,34 +117,17 @@ public void testGoogModuleGetNonStringIsError() { public void testGoogRequireForProvide() { testModules( - lines("const y = goog.require('closure.provide');", "use(y, y.x)", "export {};"), + "const y = goog.require('closure.provide');\nexport {};", lines( - "use(closure.provide, closure.provide.x);", - "/** @const */ var module$testcode = {};")); - } - - public void testGoogRequireForProvideWithDestructure() { - testModules( - lines("const {a, b:c} = goog.require('closure.provide');", "use(a, a.z, c)", "export {};"), - lines( - "use(closure.provide.a, closure.provide.a.z, closure.provide.b);", + "const y$$module$testcode = closure.provide;", "/** @const */ var module$testcode = {};")); } public void testGoogRequireForGoogModule() { testModules( - lines("const y = goog.require('closure.module');", "use(y, y.x)", "export {};"), + "const y = goog.require('closure.module');\nexport {};", lines( - "use(module$exports$closure$module, module$exports$closure$module.x);", - "/** @const */ var module$testcode = {};")); - } - - public void testGoogRequireForGoogModuleWithDestructure() { - testModules( - lines("const {a, b:c} = goog.require('closure.module');", "use(a, a.z, c)", "export {};"), - lines( - "use(module$exports$closure$module.a, module$exports$closure$module.a.z,", - " module$exports$closure$module.b);", + "const y$$module$testcode = module$exports$closure$module;", "/** @const */ var module$testcode = {};")); } @@ -158,20 +141,9 @@ public void testGoogModuleGetForGoogModule() { public void testGoogRequireForLegacyGoogModule() { testModules( - lines("const y = goog.require('closure.legacy.module');", "use(y, y.x)", "export {};"), - lines( - "use(closure.legacy.module, closure.legacy.module.x);", - "/** @const */ var module$testcode = {};")); - } - - public void testGoogRequireForLegacyGoogModuleWithDestructure() { - testModules( - lines( - "const {a, b:c} = goog.require('closure.legacy.module');", - "use(a, a.z, c)", - "export {};"), + "const y = goog.require('closure.legacy.module');\nexport {};", lines( - "use(closure.legacy.module.a, closure.legacy.module.a.z, closure.legacy.module.b);", + "const y$$module$testcode = closure.legacy.module;", "/** @const */ var module$testcode = {};")); } @@ -223,6 +195,19 @@ public void testGoogRequireLhsNonConstIsError() { testModulesError( "export var x;\nvar bar = goog.require('closure.provide');", LHS_OF_GOOG_REQUIRE_MUST_BE_CONST); + } + + public void testGoogRequiresDestructuring_rewrite() { + testModules( + lines( + "export {};", "const {foo, bar} = goog.require('closure.provide');", "use(foo, bar);"), + lines( + "const {", + " foo: foo$$module$testcode,", + " bar: bar$$module$testcode,", + "} = closure.provide;", + "use(foo$$module$testcode, bar$$module$testcode);", + "/** @const */ var module$testcode = {};")); testModulesError( lines("export {};", "var {foo, bar} = goog.require('closure.provide');", "use(foo, bar);"), diff --git a/test/com/google/javascript/jscomp/runtime_tests/module_tests/import_star_test.js b/test/com/google/javascript/jscomp/runtime_tests/module_tests/import_star_test.js index a6b1dd38eb1..08e9b168936 100644 --- a/test/com/google/javascript/jscomp/runtime_tests/module_tests/import_star_test.js +++ b/test/com/google/javascript/jscomp/runtime_tests/module_tests/import_star_test.js @@ -26,6 +26,6 @@ testSuite({ /** @suppress {checkTypes} */ testUnexportedProperty() { // Module-scoped variable, not exported with this name. - assertEquals(undefined, m['a']); + assertEquals(undefined, m.a); }, });