Skip to content

Commit

Permalink
Roll forward: Update DepsGenerator to support goog.require's with es6…
Browse files Browse the repository at this point in the history 
… modules:

- goog.require(path) is now allowed in goog.modules for ES6 modules.
- ES6 modules no longer provide any symbols in a deps file.
- Requires for ES6 modules in a deps file will be their relative path to Closure rather than a munged symbol.

NEW:

- Emit a warning if a file is any combination of goog.provide, goog.module, ES6 module. TODO for this to become an error.
- Filter out munged provides when writing ES6 modules to a deps file, leaving in place any other provided symbols. Before ES6 modules just always had no provides, no matter what.

The above means that an ES6 module with a goog.provide is technically warning and should work, for now. Once we have more official mechanisms for interop this will be promoted to an error. *Do not rely on the behavior because it will eventually be an error!*

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=187353966
  • Loading branch information
johnplaisted authored and Tyler Breisacher committed Mar 3, 2018
1 parent e359a7e commit 1c1c07d
Show file tree
Hide file tree
Showing 21 changed files with 1,007 additions and 384 deletions.
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/Compiler.java
View file
Expand Up @@ -1807,7 +1807,7 @@ Node parseInputs() {
// In this case we must force module rewriting to occur on the imported file
Map<String, CompilerInput> inputsToRewrite = new HashMap<>();
for (CompilerInput input : inputs) {
for (String require : input.getKnownRequires()) {
for (String require : input.getKnownRequiredSymbols()) {
if (inputModuleIdentifiers.containsKey(require)
&& !inputsToRewrite.containsKey(require)) {
inputsToRewrite.put(require, inputModuleIdentifiers.get(require));
Expand Down Expand Up @@ -2015,7 +2015,7 @@ private List<CompilerInput> depthFirstDependenciesFromInput(
this.moduleTypesByName.put(input.getPath().toModuleName(), input.getJsModuleType());

ArrayList<String> allDeps = new ArrayList<>();
allDeps.addAll(input.getRequires());
allDeps.addAll(input.getRequiredSymbols());
allDeps.addAll(input.getDynamicRequires());
for (String requiredNamespace : allDeps) {
CompilerInput requiredInput = null;
Expand Down
51 changes: 24 additions & 27 deletions src/com/google/javascript/jscomp/CompilerInput.java
View file
Expand Up @@ -40,12 +40,12 @@
import java.util.TreeMap;

/**
* A class for the internal representation of an input to the compiler.
* Wraps a {@link SourceAst} and maintain state such as module for the input and
* whether the input is an extern. Also calculates provided and required types.
* A class for the internal representation of an input to the compiler. Wraps a {@link SourceAst}
* and maintain state such as module for the input and whether the input is an extern. Also
* calculates provided and required types.
*
*/
public class CompilerInput implements SourceAst, DependencyInfo {
public class CompilerInput extends DependencyInfo.Base implements SourceAst {

private static final long serialVersionUID = 1L;

Expand All @@ -58,9 +58,9 @@ public class CompilerInput implements SourceAst, DependencyInfo {

// DependencyInfo to delegate to.
private DependencyInfo dependencyInfo;
private final List<String> extraRequires = new ArrayList<>();
private final List<Require> extraRequires = new ArrayList<>();
private final List<String> extraProvides = new ArrayList<>();
private final List<String> orderedRequires = new ArrayList<>();
private final List<Require> orderedRequires = new ArrayList<>();
private final List<String> dynamicRequires = new ArrayList<>();
private boolean hasFullParseDependencyInfo = false;
private ModuleType jsModuleType = ModuleType.NONE;
Expand Down Expand Up @@ -162,7 +162,7 @@ public void setCompiler(AbstractCompiler compiler) {

/** Gets a list of types depended on by this input. */
@Override
public ImmutableList<String> getRequires() {
public ImmutableList<Require> getRequires() {
if (hasFullParseDependencyInfo) {
return ImmutableList.copyOf(orderedRequires);
}
Expand All @@ -176,14 +176,16 @@ public ImmutableList<String> getWeakRequires() {
}

/**
* Gets a list of types depended on by this input,
* but does not attempt to regenerate the dependency information.
* Typically this occurs from module rewriting.
* Gets a list of namespaces and paths depended on by this input, but does not attempt to
* regenerate the dependency information. Typically this occurs from module rewriting.
*/
ImmutableCollection<String> getKnownRequires() {
ImmutableCollection<Require> getKnownRequires() {
return concat(
dependencyInfo != null ? dependencyInfo.getRequires() : ImmutableList.<String>of(),
extraRequires);
dependencyInfo != null ? dependencyInfo.getRequires() : ImmutableList.of(), extraRequires);
}

ImmutableList<String> getKnownRequiredSymbols() {
return Require.asSymbolList(getKnownRequires());
}

/** Gets a list of types provided by this input. */
Expand Down Expand Up @@ -212,7 +214,7 @@ public void addProvide(String provide) {
}

/** Registers a type that this input depends on in the order seen in the file. */
public boolean addOrderedRequire(String require) {
public boolean addOrderedRequire(Require require) {
if (!orderedRequires.contains(require)) {
orderedRequires.add(require);
return true;
Expand Down Expand Up @@ -255,7 +257,7 @@ public void setJsModuleType(ModuleType moduleType) {
}

/** Registers a type that this input depends on. */
public void addRequire(String require) {
public void addRequire(Require require) {
extraRequires.add(require);
}

Expand Down Expand Up @@ -341,7 +343,7 @@ private DependencyInfo generateDependencyInfo() {
private static class DepsFinder {
private final Map<String, String> loadFlags = new TreeMap<>();
private final List<String> provides = new ArrayList<>();
private final List<String> requires = new ArrayList<>();
private final List<Require> requires = new ArrayList<>();
private final ModulePath modulePath;

DepsFinder(ModulePath modulePath) {
Expand Down Expand Up @@ -369,14 +371,13 @@ void visitSubtree(Node n, Node parent) {
&& n.getFirstChild().isGetProp()
&& n.getFirstFirstChild().matchesQualifiedName("goog")) {

if (!requires.contains("goog")) {
requires.add("goog");
if (!requires.contains(Require.BASE)) {
requires.add(Require.BASE);
}

Node callee = n.getFirstChild();
Node argument = n.getLastChild();
switch (callee.getLastChild().getString()) {

case "module":
loadFlags.put("module", "goog");
// Fall-through
Expand All @@ -391,7 +392,7 @@ void visitSubtree(Node n, Node parent) {
if (!argument.isString()) {
return;
}
requires.add(argument.getString());
requires.add(Require.googRequireSymbol(argument.getString()));
return;

case "loadModule":
Expand Down Expand Up @@ -456,7 +457,8 @@ void visitEs6ModuleName(Node n, Node parent) {
// into ModuleLoader.
String moduleName = n.getString();
if (moduleName.startsWith("goog:")) {
requires.add(moduleName.substring(5)); // cut off the "goog:" prefix
// cut off the "goog:" prefix
requires.add(Require.googRequireSymbol(moduleName.substring(5)));
return;
}
ModulePath importedModule =
Expand All @@ -467,7 +469,7 @@ void visitEs6ModuleName(Node n, Node parent) {
importedModule = modulePath.resolveModuleAsPath(moduleName);
}

requires.add(importedModule.toModuleName());
requires.add(Require.es6Import(importedModule.toModuleName(), n.getString()));
}
}

Expand Down Expand Up @@ -525,11 +527,6 @@ public ImmutableMap<String, String> getLoadFlags() {
return getDependencyInfo().getLoadFlags();
}

@Override
public boolean isModule() {
return "goog".equals(getLoadFlags().get("module"));
}

private static <T> ImmutableSet<T> concat(Iterable<T> first, Iterable<T> second) {
return ImmutableSet.<T>builder().addAll(first).addAll(second).build();
}
Expand Down
15 changes: 8 additions & 7 deletions src/com/google/javascript/jscomp/FindModuleDependencies.java
View file
Expand Up @@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.javascript.jscomp.CompilerInput.ModuleType;
import com.google.javascript.jscomp.Es6RewriteModules.FindGoogProvideOrGoogModule;
import com.google.javascript.jscomp.deps.DependencyInfo.Require;
import com.google.javascript.jscomp.deps.ModuleLoader;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
Expand Down Expand Up @@ -73,8 +74,8 @@ public void process(Node root) {
// and add "goog" as a dependency. If "goog" is a dependency of the
// file we add it here to the ordered requires so that it's always
// first.
if (input.getRequires().contains("goog")) {
input.addOrderedRequire("goog");
if (input.getRequires().contains(Require.BASE)) {
input.addOrderedRequire(Require.BASE);
}

NodeTraversal.traverseEs6(compiler, root, this);
Expand Down Expand Up @@ -159,7 +160,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
.matchesQualifiedName("__webpack_require__.e"))) {
t.getInput().addDynamicRequire(modulePath.toModuleName());
} else {
t.getInput().addOrderedRequire(modulePath.toModuleName());
t.getInput().addOrderedRequire(Require.commonJs(modulePath.toModuleName(), path));
}
}
}
Expand All @@ -175,9 +176,9 @@ public void visit(NodeTraversal t, Node n, Node parent) {
&& n.getSecondChild().isString()) {
String namespace = n.getSecondChild().getString();
if (namespace.startsWith("goog.")) {
t.getInput().addOrderedRequire("goog");
t.getInput().addOrderedRequire(Require.BASE);
}
t.getInput().addOrderedRequire(namespace);
t.getInput().addOrderedRequire(Require.googRequireSymbol(namespace));
}
}

Expand All @@ -197,9 +198,9 @@ public void exitScope(NodeTraversal t) {
private void addEs6ModuleImportToGraph(NodeTraversal t, Node n) {
String moduleName = getEs6ModuleNameFromImportNode(t, n);
if (moduleName.startsWith("goog.")) {
t.getInput().addOrderedRequire("goog");
t.getInput().addOrderedRequire(Require.BASE);
}
t.getInput().addOrderedRequire(moduleName);
t.getInput().addOrderedRequire(Require.es6Import(moduleName, n.getLastChild().getString()));
}

/**
Expand Down
12 changes: 6 additions & 6 deletions src/com/google/javascript/jscomp/JSModule.java
View file
Expand Up @@ -35,11 +35,11 @@
import java.util.Set;

/**
* A JavaScript module has a unique name, consists of a list of compiler inputs,
* and can depend on other modules.
* A JavaScript module has a unique name, consists of a list of compiler inputs, and can depend on
* other modules.
*
*/
public final class JSModule implements DependencyInfo, Serializable {
public final class JSModule extends DependencyInfo.Base implements Serializable {
private static final long serialVersionUID = 1;

/** Module name */
Expand Down Expand Up @@ -86,10 +86,10 @@ public ImmutableList<String> getProvides() {
}

@Override
public ImmutableList<String> getRequires() {
ImmutableList.Builder<String> builder = ImmutableList.builder();
public ImmutableList<Require> getRequires() {
ImmutableList.Builder<Require> builder = ImmutableList.builder();
for (JSModule m : deps) {
builder.add(m.getName());
builder.add(Require.compilerModule(m.getName()));
}
return builder.build();
}
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/JSModuleGraph.java
View file
Expand Up @@ -532,7 +532,7 @@ private List<CompilerInput> getDepthFirstDependenciesOf(
return orderedInputs;
}

for (String importedNamespace : rootInput.getRequires()) {
for (String importedNamespace : rootInput.getRequiredSymbols()) {
CompilerInput dependency = null;
if (inputsByProvide.containsKey(importedNamespace)
&& unreachedInputs.contains(inputsByProvide.get(importedNamespace))) {
Expand Down
13 changes: 3 additions & 10 deletions src/com/google/javascript/jscomp/LazyParsedDependencyInfo.java
View file
Expand Up @@ -27,10 +27,8 @@
import java.util.Map;
import java.util.TreeMap;

/**
* A DependencyInfo class that determines load flags by parsing the AST just-in-time.
*/
public class LazyParsedDependencyInfo implements DependencyInfo {
/** A DependencyInfo class that determines load flags by parsing the AST just-in-time. */
public class LazyParsedDependencyInfo extends DependencyInfo.Base {

private final DependencyInfo delegate;
private final JsAst ast;
Expand Down Expand Up @@ -77,7 +75,7 @@ public String getPathRelativeToClosureBase() {
}

@Override
public ImmutableList<String> getRequires() {
public ImmutableList<Require> getRequires() {
return delegate.getRequires();
}

Expand All @@ -90,9 +88,4 @@ public ImmutableList<String> getWeakRequires() {
public ImmutableList<String> getProvides() {
return delegate.getProvides();
}

@Override
public boolean isModule() {
return "goog".equals(getLoadFlags().get("module"));
}
}
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/deps/DefaultDependencyResolver.java
View file
Expand Up @@ -131,7 +131,7 @@ private void addDependency(String symbol, Set<String> seen, List<String> list)
}
} else if (!seen.containsAll(dependency.getProvides())) {
seen.addAll(dependency.getProvides());
for (String require : dependency.getRequires()) {
for (String require : dependency.getRequiredSymbols()) {
addDependency(require, seen, list);
}
list.add(dependency.getPathRelativeToClosureBase());
Expand All @@ -148,7 +148,7 @@ private static Collection<String> parseRequires(String code, boolean addClosureB
if (addClosureBase) {
requires.add(CLOSURE_BASE_PROVIDE);
}
requires.addAll(deps.getRequires());
requires.addAll(deps.getRequiredSymbols());
errorManager.generateReport();
return requires;
}
Expand Down

0 comments on commit 1c1c07d

Please sign in to comment.