Skip to content

Commit

Permalink
Add an ES6 module's relative path to closure as an implicit provide w…
Browse files Browse the repository at this point in the history
…hen parsing deps.js files so that dependency graphs are correct.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=190859615
  • Loading branch information
johnplaisted authored and Tyler Breisacher committed Mar 30, 2018
1 parent 9c4ac84 commit 7ed7854
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 7 deletions.
18 changes: 13 additions & 5 deletions src/com/google/javascript/jscomp/deps/DepsFileParser.java
Expand Up @@ -48,7 +48,6 @@
*/ */
@GwtIncompatible("java.util.regex") @GwtIncompatible("java.util.regex")
public final class DepsFileParser extends JsFileLineParser { public final class DepsFileParser extends JsFileLineParser {

private static final Logger logger = Logger.getLogger(DepsFileParser.class.getName()); private static final Logger logger = Logger.getLogger(DepsFileParser.class.getName());


/** /**
Expand Down Expand Up @@ -82,7 +81,7 @@ public final class DepsFileParser extends JsFileLineParser {
* @param errorManager Handles parse errors. * @param errorManager Handles parse errors.
*/ */
public DepsFileParser(ErrorManager errorManager) { public DepsFileParser(ErrorManager errorManager) {
this(Functions.<String>identity(), errorManager); this(Functions.identity(), errorManager);
} }


/** /**
Expand Down Expand Up @@ -166,17 +165,26 @@ protected boolean parseLine(String line) throws ParseException {
+ addDependencyParams, true); + addDependencyParams, true);
} }
// Parse the file path. // Parse the file path.
String path = pathTranslator.apply(parseJsString(depArgsMatch.group(1))); String relativePath = parseJsString(depArgsMatch.group(1));
String path = pathTranslator.apply(relativePath);

List<String> provides = parseJsStringArray(depArgsMatch.group(2));
Map<String, String> loadFlags = parseLoadFlags(depArgsMatch.group(4));

// ES6 modules are require'd by path but do not provide them in the addDependency call.
if ("es6".equals(loadFlags.get("module"))) {
provides.add(relativePath);
}


DependencyInfo depInfo = DependencyInfo depInfo =
SimpleDependencyInfo.builder(path, filePath) SimpleDependencyInfo.builder(path, filePath)
.setProvides(parseJsStringArray(depArgsMatch.group(2))) .setProvides(provides)
.setRequires( .setRequires(
parseJsStringArray(depArgsMatch.group(3)) parseJsStringArray(depArgsMatch.group(3))
.stream() .stream()
.map(Require::parsedFromDeps) .map(Require::parsedFromDeps)
.collect(toImmutableList())) .collect(toImmutableList()))
.setLoadFlags(parseLoadFlags(depArgsMatch.group(4))) .setLoadFlags(loadFlags)
.build(); .build();


if (logger.isLoggable(Level.FINE)) { if (logger.isLoggable(Level.FINE)) {
Expand Down
14 changes: 13 additions & 1 deletion src/com/google/javascript/jscomp/deps/DepsGenerator.java
Expand Up @@ -47,6 +47,7 @@
import java.util.Set; import java.util.Set;
import java.util.logging.Level; import java.util.logging.Level;
import java.util.logging.Logger; import java.util.logging.Logger;
import java.util.stream.Collectors;


/** /**
* Generates deps.js files by scanning JavaScript files for * Generates deps.js files by scanning JavaScript files for
Expand Down Expand Up @@ -477,7 +478,18 @@ private Map<String, DependencyInfo> parseDepsFiles() throws IOException {
reportNoDepsInDepsFile(file.getName()); reportNoDepsInDepsFile(file.getName());
} else { } else {
for (DependencyInfo info : depInfos) { for (DependencyInfo info : depInfos) {
depsFiles.put(info.getPathRelativeToClosureBase(), info); // DepsFileParser adds an ES6 module's relative path to closure as a provide so that
// the resulting depgraph is valid. But we don't want to write this "fake" provide
// back out, so remove it here.
depsFiles.put(
info.getPathRelativeToClosureBase(),
SimpleDependencyInfo.Builder.from(info)
.setProvides(
info.getProvides()
.stream()
.filter(p -> !p.equals(info.getPathRelativeToClosureBase()))
.collect(Collectors.toList()))
.build());
} }
} }
} }
Expand Down
10 changes: 10 additions & 0 deletions src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java
Expand Up @@ -45,6 +45,16 @@ public static Builder builder(String srcPathRelativeToClosure, String pathOfDefi
*/ */
@AutoValue.Builder @AutoValue.Builder
public abstract static class Builder { public abstract static class Builder {
public static Builder from(DependencyInfo copy) {
return new AutoValue_SimpleDependencyInfo.Builder()
.setName(copy.getName())
.setPathRelativeToClosureBase(copy.getPathRelativeToClosureBase())
.setProvides(copy.getProvides())
.setRequires(copy.getRequires())
.setWeakRequires(copy.getWeakRequires())
.setLoadFlags(copy.getLoadFlags());
}

abstract Builder setName(String name); abstract Builder setName(String name);
abstract Builder setPathRelativeToClosureBase(String srcPathRelativeToClosure); abstract Builder setPathRelativeToClosureBase(String srcPathRelativeToClosure);


Expand Down
23 changes: 22 additions & 1 deletion test/com/google/javascript/jscomp/deps/DepsFileParserTest.java
Expand Up @@ -115,7 +115,7 @@ public void testBadLoadFlagsSyntax() {
assertThat(errorManager.getWarningCount()).isEqualTo(0); assertThat(errorManager.getWarningCount()).isEqualTo(0);
} }


public void testModule() { public void testGoogModule() {
List<DependencyInfo> result = parser.parseFile(SRC_PATH, List<DependencyInfo> result = parser.parseFile(SRC_PATH,
"goog.addDependency('yes1', [], [], true);\n" + "goog.addDependency('yes1', [], [], true);\n" +
"goog.addDependency('yes2', [], [], false);\n"); "goog.addDependency('yes2', [], [], false);\n");
Expand All @@ -126,6 +126,27 @@ public void testModule() {
assertThat(result).isEqualTo(expected); assertThat(result).isEqualTo(expected);
} }


public void testEs6Module() {
List<DependencyInfo> result =
parser.parseFile(
SRC_PATH,
"goog.addDependency('path/from/closure.js', [], ['nexttoclosure.js'], "
+ "{'module':'es6'});\n"
+ "goog.addDependency('nexttoclosure.js', [], [], {'module':'es6'});\n");
ImmutableList<DependencyInfo> expected =
ImmutableList.of(
SimpleDependencyInfo.builder("path/from/closure.js", SRC_PATH)
.setLoadFlags(ImmutableMap.of("module", "es6"))
.setProvides(ImmutableList.of("path/from/closure.js"))
.setRequires(Require.parsedFromDeps("nexttoclosure.js"))
.build(),
SimpleDependencyInfo.builder("nexttoclosure.js", SRC_PATH)
.setLoadFlags(ImmutableMap.of("module", "es6"))
.setProvides(ImmutableList.of("nexttoclosure.js"))
.build());
assertThat(result).isEqualTo(expected);
}

public void testLoadFlags() { public void testLoadFlags() {
List<DependencyInfo> result = parser.parseFile(SRC_PATH, "" List<DependencyInfo> result = parser.parseFile(SRC_PATH, ""
+ "goog.addDependency('yes1', [], [], {'module': 'goog'});\n" + "goog.addDependency('yes1', [], [], {'module': 'goog'});\n"
Expand Down

0 comments on commit 7ed7854

Please sign in to comment.