Skip to content

Commit

Permalink
Enable goog.require/module.get/forwardDeclare for ES6 modules *in* ES…
Browse files Browse the repository at this point in the history 
…6 modules.

These functions currently work for ES6 modules in non-ES6 modules only.

Also while we're here report a warning of an ES6 module goog.requires an ES6 module. While this works, it really aught to use the import syntax.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=214646452
  • Loading branch information
@brad4d
johnplaisted authored and brad4d committed Sep 27, 2018
1 parent bb05a1c commit 03b7f18
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 13 deletions.
40 changes: 30 additions & 10 deletions src/com/google/javascript/jscomp/Es6RewriteModules.java
View file
Expand Up @@ -77,10 +77,15 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback
static final DiagnosticType DUPLICATE_EXPORT =
DiagnosticType.error("JSC_DUPLICATE_EXPORT", "Duplicate export ''{0}''.");

static final DiagnosticType FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST =
static final DiagnosticType FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST =
DiagnosticType.error(
"JSC_FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST",
"Aliases of goog.forwardDeclare() in an ES6 module should be const.");
"JSC_FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST",
"goog.forwardDeclare alias for ES6 module should be const.");

static final DiagnosticType SHOULD_IMPORT_ES6_MODULE =
DiagnosticType.warning(
"JSC_SHOULD_IMPORT_ES6_MODULE",
"ES6 modules should import other ES6 modules rather than goog.require them.");

private final AbstractCompiler compiler;

Expand Down Expand Up @@ -178,10 +183,9 @@ public void process(Node externs, Node root) {

@Override
public void hotSwapScript(Node scriptNode, Node originalRoot) {
new RewriteRequiresForEs6Modules().rewrite(scriptNode);
if (isEs6ModuleRoot(scriptNode)) {
processFile(scriptNode);
} else {
new RewriteRequiresForEs6Modules().rewrite(scriptNode);
}
}

Expand Down Expand Up @@ -254,6 +258,17 @@ public void visit(NodeTraversal t, Node n, Node parent) {
return;
}

// TODO(johnplaisted): Once we have an alternative to forwardDeclare / requireType that
// doesn't require Closure Library warn about those too.
// TODO(johnplaisted): Once we have import() support warn about goog.module.get.
if (isRequire) {
ModuleMetadata currentModuleMetadata =
moduleMetadataMap.getModulesByPath().get(t.getInput().getPath().toString());
if (currentModuleMetadata != null && currentModuleMetadata.isEs6Module()) {
t.report(n, SHOULD_IMPORT_ES6_MODULE);
}
}

if (isGet && t.inGlobalHoistScope()) {
t.report(n, INVALID_GET_CALL_SCOPE);
return;
Expand Down Expand Up @@ -297,7 +312,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
statementNode.detach();
t.reportCodeChange();
} else {
t.report(statementNode, FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST);
t.report(statementNode, FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
}
} else {
// const module = goog.require('an.es6.namespace');
Expand All @@ -312,11 +327,16 @@ public void visit(NodeTraversal t, Node n, Node parent) {
if (isForwardDeclare) {
forwardDeclareRenameTable.put(
t.getScopeRoot(), name, ModuleRenaming.getGlobalName(moduleMetadata, name));
statementNode.detach();
} else {
if (statementNode.isExprResult() && statementNode.getFirstChild() == n) {
statementNode.detach();
} else {
n.replaceWith(
IR.name(ModuleRenaming.getGlobalName(moduleMetadata, name))
.useSourceInfoFromForTree(n));
}
}
if (parent.isExprResult()) {
parent.detach();
}
n.detach();
t.reportCodeChange();
}

Expand Down
126 changes: 123 additions & 3 deletions test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java
View file
Expand Up @@ -20,7 +20,7 @@
import static com.google.javascript.jscomp.ClosurePrimitiveErrors.INVALID_GET_NAMESPACE;
import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MISSING_MODULE_OR_PROVIDE;
import static com.google.javascript.jscomp.ClosurePrimitiveErrors.MODULE_USES_GOOG_MODULE_GET;
import static com.google.javascript.jscomp.Es6RewriteModules.FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST;
import static com.google.javascript.jscomp.Es6RewriteModules.FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST;
import static com.google.javascript.jscomp.Es6RewriteModules.LHS_OF_GOOG_REQUIRE_MUST_BE_CONST;
import static com.google.javascript.jscomp.Es6RewriteModules.NAMESPACE_IMPORT_CANNOT_USE_STAR;

Expand Down Expand Up @@ -230,6 +230,22 @@ public void testGoogModuleGetForEs6ModuleDeclaresNamespace() {
SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
SourceFile.fromCode(
"closure.js", "goog.module('my.module'); function f() { const y = module$es6; }")));

test(
srcs(
SourceFile.fromCode("es6.js", "export let y; goog.module.declareNamespace('es6');"),
SourceFile.fromCode(
"closure.js",
"goog.module('my.module'); function f() { return goog.module.get('es6').y; }")),
expected(
SourceFile.fromCode(
"es6.js",
lines(
"let y$$module$es6;",
"/** @const */ var module$es6 = {};",
"/** @const */ module$es6.y = y$$module$es6;")),
SourceFile.fromCode(
"closure.js", "goog.module('my.module'); function f() { return module$es6.y; }")));
}

@Test
Expand Down Expand Up @@ -474,7 +490,7 @@ public void testForwardDeclareEs6Module() {
"let alias = goog.forwardDeclare('es6');",
"let /** !alias.Type */ x;",
"alias = goog.modle.get('es6');"))),
FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST);
FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
}

@Test
Expand Down Expand Up @@ -521,6 +537,110 @@ public void testForwardDeclareEs6ModuleDeclareNamespace() {
"let alias = goog.forwardDeclare('es6');",
"let /** !alias.Type */ x;",
"alias = goog.modle.get('es6');"))),
FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST);
FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
}

@Test
public void testWarnAboutRequireEs6FromEs6() {
test(
srcs(
SourceFile.fromCode("first.js", "goog.module.declareNamespace('first'); export {};"),
SourceFile.fromCode(
"second.js", "const first = goog.require('first'); export {first};")),
expected(
SourceFile.fromCode("first.js", "/** @const */ var module$first = {};"),
SourceFile.fromCode(
"second.js",
lines(
"const first$$module$second = module$first;",
"/** @const */ var module$second = {};",
"/** @const */ module$second.first = first$$module$second;"))),
warning(Es6RewriteModules.SHOULD_IMPORT_ES6_MODULE));

test(
srcs(
SourceFile.fromCode("first.js", "goog.module.declareNamespace('no.alias'); export {};"),
SourceFile.fromCode("second.js", "goog.require('no.alias'); export {};")),
expected(
SourceFile.fromCode("first.js", "/** @const */ var module$first = {};"),
SourceFile.fromCode("second.js", "/** @const */ var module$second = {};")),
warning(Es6RewriteModules.SHOULD_IMPORT_ES6_MODULE));
}

@Test
public void testGoogModuleGetEs6ModuleInEs6Module() {
test(
srcs(
SourceFile.fromCode("first.js", "goog.module.declareNamespace('first'); export let x;"),
SourceFile.fromCode(
"second.js", "export function foo() { return goog.module.get('first').x; }")),
expected(
SourceFile.fromCode(
"first.js",
lines(
"let x$$module$first;",
"/** @const */ var module$first = {};",
"/** @const */ module$first.x = x$$module$first;")),
SourceFile.fromCode(
"second.js",
lines(
"function foo$$module$second() {",
" return module$first.x;",
"}",
"/** @const */ var module$second = {};",
"/** @const */ module$second.foo = foo$$module$second;"))));
}

@Test
public void testForwardDeclareEs6ModuleInEs6Module() {
test(
srcs(
SourceFile.fromCode("es6.js", "export {}; goog.module.declareNamespace('es6');"),
SourceFile.fromCode(
"forwarddeclare.js",
lines(
"export {}",
"const alias = goog.forwardDeclare('es6');",
"let /** !alias.Type */ x;",
"alias;"))),
expected(
SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
SourceFile.fromCode(
"forwarddeclare.js",
lines(
"let /** !module$es6.Type */ x$$module$forwarddeclare;",
"alias;",
"/** @const */ var module$forwarddeclare = {};"))));

test(
srcs(
SourceFile.fromCode("es6.js", "export{}; goog.module.declareNamespace('es6');"),
SourceFile.fromCode(
"forwarddeclare.js",
lines(
"export {};",
"goog.forwardDeclare('es6');",
"let /** !es6.Type */ x;",
"es6;"))),
expected(
SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
SourceFile.fromCode(
"closure.js",
lines(
"let /** !module$es6.Type */ x$$module$forwarddeclare;",
"es6;",
"/** @const */ var module$forwarddeclare = {};"))));

testError(
ImmutableList.of(
SourceFile.fromCode("es6.js", "export{}; goog.module.declareNamespace('es6');"),
SourceFile.fromCode(
"closure.js",
lines(
"export {};",
"let alias = goog.forwardDeclare('es6');",
"let /** !alias.Type */ x;",
"alias = goog.modle.get('es6');"))),
FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST);
}
}

0 comments on commit 03b7f18

Please sign in to comment.