Skip to content

Commit

Permalink
Rollback "Inline require'd symbols when rewriting ES6 modules."
Browse files Browse the repository at this point in the history 
It broke internal lit_element.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=205871049
  • Loading branch information
@brad4d
brad4d committed Jul 25, 2018
1 parent 626225c commit 30250ac
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 110 deletions.
82 changes: 7 additions & 75 deletions src/com/google/javascript/jscomp/Es6RewriteModules.java
View file
Expand Up @@ -103,36 +103,6 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback
*/
private Map<String, ModuleOriginalNamePair> 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.
*
* <p>We use this to rewrite something like:
*
* <pre>
* import {x} from '';
* const {assert} = goog.require('goog.asserts');
* assert(x);
* </pre>
*
* To:
*
* <pre>
* import {x} from '';
* goog.asserts.assert(x);
* </pre>
*
* Because if we used an alias like below the assertion would not be recognized:
*
* <pre>
* import {x} from '';
* const {assert} = goog.asserts;
* assert(x);
* </pre>
*/
private Map<String, String> namesToInlineByAlias;

private final Set<Node> importedGoogNames;

private Set<String> typedefs;
Expand Down Expand Up @@ -209,7 +179,6 @@ public void clearState() {
this.importMap = new HashMap<>();
this.importedGoogNames.clear();
this.typedefs = new HashSet<>();
this.namesToInlineByAlias = new HashMap<>();
}

/**
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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();
Expand Down
53 changes: 19 additions & 34 deletions test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java
View file
Expand Up @@ -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 = {};"));
}

Expand All @@ -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 = {};"));
}

Expand Down Expand Up @@ -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);"),
Expand Down
2 changes: 1 addition & 1 deletion test/com/google/javascript/jscomp/runtime_tests/module_tests/import_star_test.js
View file
Expand Up @@ -26,6 +26,6 @@ testSuite({
/** @suppress {checkTypes} */
testUnexportedProperty() {
// Module-scoped variable, not exported with this name.
assertEquals(undefined, m['a']);
assertEquals(undefined, m.a);
},
});

0 comments on commit 30250ac

Please sign in to comment.