From c668ed761b506a435ae71ede9fa9e06c19d307ef Mon Sep 17 00:00:00 2001 From: Chad Killingsworth Date: Thu, 7 Sep 2017 19:18:25 -0500 Subject: [PATCH 1/2] When --dependency_mode is set to STRICT or LOOSE, order dependencies in a deterministic depth-first order from entry points. Avoids a full parse on every input when CommonJS module processing is enabled. ES6 and CommonJS modules no longer generate synthetic goog.require and goog.provide calls. ES6 Module Transpilation no longer depends on Closure-Library primitives. --- .../google/javascript/jscomp/Compiler.java | 172 ++++++++++++++-- .../javascript/jscomp/CompilerInput.java | 45 ++++- .../javascript/jscomp/DependencyOptions.java | 4 +- .../javascript/jscomp/Es6RewriteModules.java | 59 +----- .../jscomp/FindModuleDependencies.java | 183 ++++++++++++++++++ .../javascript/jscomp/JSModuleGraph.java | 86 ++++++-- .../jscomp/ProcessCommonJSModules.java | 168 +++++++--------- .../jscomp/CommandLineRunnerTest.java | 51 +++-- .../javascript/jscomp/CompilerTest.java | 103 ++++++++++ .../jscomp/Es6RewriteModulesTest.java | 142 ++++++-------- .../javascript/jscomp/JSModuleGraphTest.java | 101 ++++++++++ .../jscomp/ProcessCommonJSModulesTest.java | 133 ++----------- 12 files changed, 825 insertions(+), 422 deletions(-) create mode 100644 src/com/google/javascript/jscomp/FindModuleDependencies.java diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 94ff7c94181..046c1186334 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -64,7 +64,6 @@ import java.io.Serializable; import java.util.AbstractSet; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -1749,7 +1748,19 @@ Node parseInputs() { options.moduleResolutionMode, processJsonInputs(inputs)); } + } else { + // Use an empty module loader if we're not actually dealing with modules. + this.moduleLoader = ModuleLoader.EMPTY; + } + if (options.getDependencyOptions().needsManagement()) { + findDependenciesFromEntryPoints( + options.getLanguageIn().toFeatureSet().has(Feature.MODULES), + options.processCommonJSModules, + options.transformAMDToCJSModules); + } else if (options.needsTranspilationFrom(FeatureSet.ES6_MODULES) + || options.transformAMDToCJSModules + || options.processCommonJSModules) { if (options.getLanguageIn().toFeatureSet().has(Feature.MODULES)) { parsePotentialModules(inputs); } @@ -1782,12 +1793,12 @@ Node parseInputs() { } } - if (!inputsToRewrite.isEmpty()) { - forceToEs6Modules(inputsToRewrite.values()); + for (CompilerInput input : inputsToRewrite.values()) { + forceInputToPathBasedModule( + input, + options.getLanguageIn().toFeatureSet().has(Feature.MODULES), + options.processCommonJSModules); } - } else { - // Use an empty module loader if we're not actually dealing with modules. - this.moduleLoader = ModuleLoader.EMPTY; } orderInputs(); @@ -1894,6 +1905,141 @@ void orderInputs() { } } + /** + * Find dependencies by recursively traversing each dependency of an input starting with the entry + * points. Causes a full parse of each file, but since the file is reachable by walking the graph, + * this would be required in later compilation passes regardless. + * + *

Inputs which are not reachable during graph traversal will be dropped. + * + *

If the dependency mode is set to LOOSE, inputs for which the deps package did not find a + * provide statement or detect as a module will be treated as entry points. + */ + void findDependenciesFromEntryPoints( + boolean supportEs6Modules, boolean supportCommonJSModules, boolean supportAmdModules) { + hoistExterns(); + List entryPoints = new ArrayList<>(); + Map inputsByProvide = new HashMap<>(); + Map inputsByIdentifier = new HashMap<>(); + for (CompilerInput input : inputs) { + if (!options.getDependencyOptions().shouldDropMoochers() && input.getProvides().isEmpty()) { + entryPoints.add(input); + } + inputsByIdentifier.put( + ModuleIdentifier.forFile(input.getPath().toString()).toString(), input); + for (String provide : input.getProvides()) { + if (!provide.startsWith("module$")) { + inputsByProvide.put(provide, input); + } + } + } + for (ModuleIdentifier moduleIdentifier : options.getDependencyOptions().getEntryPoints()) { + CompilerInput input = inputsByIdentifier.get(moduleIdentifier.toString()); + if (input != null) { + entryPoints.add(input); + } + } + + Set workingInputSet = new HashSet<>(inputs); + List orderedInputs = new ArrayList<>(); + for (CompilerInput entryPoint : entryPoints) { + orderedInputs.addAll( + depthFirstDependenciesFromInput( + entryPoint, + false, + workingInputSet, + inputsByIdentifier, + inputsByProvide, + supportEs6Modules, + supportCommonJSModules, + supportAmdModules)); + } + + // TODO(ChadKillingsworth) Move this into the standard compilation passes + if (supportCommonJSModules) { + for (CompilerInput input : orderedInputs) { + new ProcessCommonJSModules(this).process(null, input.getAstRoot(this), false); + } + } + } + + /** For a given input, order it's dependencies in a depth first traversal */ + List depthFirstDependenciesFromInput( + CompilerInput input, + boolean wasImportedByModule, + Set inputs, + Map inputsByIdentifier, + Map inputsByProvide, + boolean supportEs6Modules, + boolean supportCommonJSModules, + boolean supportAmdModules) { + List orderedInputs = new ArrayList<>(); + if (!inputs.remove(input)) { + // It's possible for a module to be included as both a script + // and a module in the same compilation. In these cases, it should + // be forced to be a module. + if (wasImportedByModule && input.getJsModuleType() == CompilerInput.ModuleType.NONE) { + forceInputToPathBasedModule(input, supportEs6Modules, supportCommonJSModules); + } + + return orderedInputs; + } + + if (supportAmdModules) { + new TransformAMDToCJSModule(this).process(null, input.getAstRoot(this)); + } + + FindModuleDependencies findDeps = + new FindModuleDependencies(this, supportEs6Modules, supportCommonJSModules); + findDeps.process(input.getAstRoot(this)); + + // If this input was imported by another module, it is itself a module + // so we force it to be detected as such. + if (wasImportedByModule && input.getJsModuleType() == CompilerInput.ModuleType.NONE) { + forceInputToPathBasedModule(input, supportEs6Modules, supportCommonJSModules); + } + + for (String requiredNamespace : input.getRequires()) { + CompilerInput requiredInput = null; + boolean requiredByModuleImport = false; + if (inputsByProvide.containsKey(requiredNamespace)) { + requiredInput = inputsByProvide.get(requiredNamespace); + } else if (inputsByIdentifier.containsKey(requiredNamespace)) { + requiredByModuleImport = true; + requiredInput = inputsByIdentifier.get(requiredNamespace); + } + + if (requiredInput != null) { + orderedInputs.addAll( + depthFirstDependenciesFromInput( + requiredInput, + requiredByModuleImport, + inputs, + inputsByIdentifier, + inputsByProvide, + supportEs6Modules, + supportCommonJSModules, + supportAmdModules)); + } + } + orderedInputs.add(input); + return orderedInputs; + } + + private void forceInputToPathBasedModule( + CompilerInput input, boolean supportEs6Modules, boolean supportCommonJSModules) { + + if (supportEs6Modules) { + FindModuleDependencies findDeps = + new FindModuleDependencies(this, supportEs6Modules, supportCommonJSModules); + findDeps.convertToEs6Module(input.getAstRoot(this)); + input.setJsModuleType(CompilerInput.ModuleType.ES6); + } else if (supportCommonJSModules) { + new ProcessCommonJSModules(this).process(null, input.getAstRoot(this), true); + input.setJsModuleType(CompilerInput.ModuleType.COMMONJS); + } + } + /** * Hoists inputs with the @externs annotation into the externs list. */ @@ -2041,18 +2187,6 @@ Map processJsonInputs(List inputsToProcess) { return rewriteJson.getPackageJsonMainEntries(); } - void forceToEs6Modules(Collection inputsToProcess) { - for (CompilerInput input : inputsToProcess) { - input.setCompiler(this); - input.addProvide(input.getPath().toModuleName()); - Node root = input.getAstRoot(this); - if (root == null) { - continue; - } - Es6RewriteModules moduleRewriter = new Es6RewriteModules(this); - moduleRewriter.forceToEs6Module(root); - } - } private List parsePotentialModules(List inputsToProcess) { List filteredInputs = new ArrayList<>(); @@ -2091,7 +2225,7 @@ void processAMDAndCommonJSModules() { new TransformAMDToCJSModule(this).process(null, root); } if (options.processCommonJSModules) { - ProcessCommonJSModules cjs = new ProcessCommonJSModules(this, true); + ProcessCommonJSModules cjs = new ProcessCommonJSModules(this); cjs.process(null, root); } } diff --git a/src/com/google/javascript/jscomp/CompilerInput.java b/src/com/google/javascript/jscomp/CompilerInput.java index 4b607a3bc06..ccd4fbd948e 100644 --- a/src/com/google/javascript/jscomp/CompilerInput.java +++ b/src/com/google/javascript/jscomp/CompilerInput.java @@ -61,6 +61,9 @@ public class CompilerInput implements SourceAst, DependencyInfo { private DependencyInfo dependencyInfo; private final List extraRequires = new ArrayList<>(); private final List extraProvides = new ArrayList<>(); + private final List orderedRequires = new ArrayList<>(); + private boolean hasFullParseDependencyInfo = false; + private ModuleType jsModuleType = ModuleType.NONE; // An AbstractCompiler for doing parsing. // We do not want to persist this across serialized state. @@ -151,6 +154,10 @@ public void setCompiler(AbstractCompiler compiler) { /** Gets a list of types depended on by this input. */ @Override public Collection getRequires() { + if (hasFullParseDependencyInfo) { + return orderedRequires; + } + return getDependencyInfo().getRequires(); } @@ -182,19 +189,36 @@ Collection getKnownProvides() { extraProvides); } - // TODO(nicksantos): Remove addProvide/addRequire/removeRequire once - // there is better support for discovering non-closure dependencies. - /** - * Registers a type that this input defines. + * Registers a type that this input defines. Includes both explicitly declared namespaces via + * goog.provide and goog.module calls as well as implicit namespaces provided by module rewriting. */ public void addProvide(String provide) { extraProvides.add(provide); } - /** - * Registers a type that this input depends on. - */ + /** Registers a type that this input depends on in the order seen in the file. */ + public boolean addOrderedRequire(String require) { + if (!orderedRequires.contains(require)) { + orderedRequires.add(require); + return true; + } + return false; + } + + public void setHasFullParseDependencyInfo(boolean hasFullParseDependencyInfo) { + this.hasFullParseDependencyInfo = hasFullParseDependencyInfo; + } + + public ModuleType getJsModuleType() { + return jsModuleType; + } + + public void setJsModuleType(ModuleType moduleType) { + jsModuleType = moduleType; + } + + /** Registers a type that this input depends on. */ public void addRequire(String require) { extraRequires.add(require); } @@ -484,4 +508,11 @@ ModulePath getPath() { public void reset() { this.module = null; } + + public enum ModuleType { + NONE, + GOOG_MODULE, + ES6, + COMMONJS + } } diff --git a/src/com/google/javascript/jscomp/DependencyOptions.java b/src/com/google/javascript/jscomp/DependencyOptions.java index 55b69576f21..21a66de1eb4 100644 --- a/src/com/google/javascript/jscomp/DependencyOptions.java +++ b/src/com/google/javascript/jscomp/DependencyOptions.java @@ -20,7 +20,7 @@ import java.io.Serializable; import java.util.Collection; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; /** @@ -43,7 +43,7 @@ public final class DependencyOptions implements Serializable { private boolean sortDependencies = false; private boolean pruneDependencies = false; private boolean dropMoochers = false; - private final Set entryPoints = new HashSet<>(); + private final Set entryPoints = new LinkedHashSet<>(); /** * Enables or disables dependency sorting mode. diff --git a/src/com/google/javascript/jscomp/Es6RewriteModules.java b/src/com/google/javascript/jscomp/Es6RewriteModules.java index 2e59243636e..3549e1f177a 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteModules.java +++ b/src/com/google/javascript/jscomp/Es6RewriteModules.java @@ -21,7 +21,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableSet; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.rhino.IR; @@ -80,8 +79,6 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback private Set alreadyRequired; - private Node googRequireInsertSpot; - /** * Creates a new Es6RewriteModules instance which can be used to rewrite * ES6 modules to a concatenable form. @@ -101,29 +98,6 @@ public static boolean isEs6ModuleRoot(Node scriptNode) { return scriptNode.hasChildren() && scriptNode.getFirstChild().isModuleBody(); } - /** - * Force rewriting of a file into an ES6 module, such as for imported files that contain no - * "import" or "export" statements. Fails if the file contains a goog.provide or goog.module. - * - * @return True, if the file is now an ES6 module. False, if the file must remain a script. - * TODO(blickly): Move this logic out of this pass, since it is independent of whether or - * not we are actually transpiling modules - */ - public boolean forceToEs6Module(Node root) { - if (isEs6ModuleRoot(root)) { - return true; - } - FindGoogProvideOrGoogModule finder = new FindGoogProvideOrGoogModule(); - NodeTraversal.traverseEs6(compiler, root, finder); - if (finder.isFound()) { - return false; - } - Node moduleNode = new Node(Token.MODULE_BODY).srcref(root); - moduleNode.addChildrenToBack(root.removeChildren()); - root.addChildToBack(moduleNode); - return true; - } - @Override public void process(Node externs, Node root) { for (Node file = root.getFirstChild(); file != null; file = file.getNext()) { @@ -155,7 +129,6 @@ public void clearState() { this.classes = new HashSet<>(); this.typedefs = new HashSet<>(); this.alreadyRequired = new HashSet<>(); - this.googRequireInsertSpot = null; } /** @@ -270,16 +243,7 @@ private void visitImport(NodeTraversal t, Node importDecl, Node parent) { } } - // Emit goog.require call for the module. - if (alreadyRequired.add(moduleName)) { - Node require = IR.exprResult( - IR.call(NodeUtil.newQName(compiler, "goog.require"), IR.string(moduleName))); - require.useSourceInfoIfMissingFromForTree(importDecl); - parent.addChildAfter(require, googRequireInsertSpot); - googRequireInsertSpot = require; - t.getInput().addRequire(moduleName); - } - + alreadyRequired.add(moduleName); parent.removeChild(importDecl); t.reportCodeChange(); } @@ -466,22 +430,13 @@ private void visitScript(NodeTraversal t, Node script) { // Rename vars to not conflict in global scope. NodeTraversal.traverseEs6(compiler, script, new RenameGlobalVars(moduleName)); - // Add goog.provide call. - Node googProvide = IR.exprResult( - IR.call(NodeUtil.newQName(compiler, "goog.provide"), - IR.string(moduleName))); - script.addChildToFront(googProvide.useSourceInfoIfMissingFromForTree(script)); - t.getInput().addProvide(moduleName); - - JSDocInfoBuilder jsDocInfo = script.getJSDocInfo() == null - ? new JSDocInfoBuilder(false) - : JSDocInfoBuilder.copyFrom(script.getJSDocInfo()); - if (!jsDocInfo.isPopulatedWithFileOverview()) { - jsDocInfo.recordFileOverview(""); + if (!exportMap.isEmpty()) { + Node moduleVar = IR.var(IR.name(moduleName), IR.objectlit()); + JSDocInfoBuilder infoBuilder = new JSDocInfoBuilder(false); + infoBuilder.recordConstancy(); + moduleVar.setJSDocInfo(infoBuilder.build()); + script.addChildToFront(moduleVar.useSourceInfoIfMissingFromForTree(script)); } - // Don't check provides and requires, since most of them are auto-generated. - jsDocInfo.recordSuppressions(ImmutableSet.of("missingProvide", "missingRequire")); - script.setJSDocInfo(jsDocInfo.build()); exportMap.clear(); t.reportCodeChange(); diff --git a/src/com/google/javascript/jscomp/FindModuleDependencies.java b/src/com/google/javascript/jscomp/FindModuleDependencies.java new file mode 100644 index 00000000000..5444fdb6931 --- /dev/null +++ b/src/com/google/javascript/jscomp/FindModuleDependencies.java @@ -0,0 +1,183 @@ +/* + * Copyright 2017 The Closure Compiler Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.javascript.jscomp; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.javascript.jscomp.Es6RewriteModules.FindGoogProvideOrGoogModule; +import com.google.javascript.jscomp.deps.ModuleLoader; +import com.google.javascript.jscomp.CompilerInput.ModuleType; +import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.Token; + +/** + * Find and update any direct dependencies of an input. Used to walk the dependency graph and + * support a strict depth-first dependency ordering. Marks an input as providing its module name. + * + *

Discovers dependencies from: - goog.require calls - ES6 import statements - CommonJS require + * statements + * + *

The order of dependency references is preserved so that a deterministic depth-first ordering + * can be achieved. + * + * @author chadkillingsworth@gmail.com (Chad Killingsworth) + */ +public class FindModuleDependencies implements NodeTraversal.Callback { + private final AbstractCompiler compiler; + private final boolean supportsEs6Modules; + private final boolean supportsCommonJsModules; + private ModuleType moduleType = ModuleType.NONE; + + FindModuleDependencies( + AbstractCompiler compiler, boolean supportsEs6Modules, boolean supportsCommonJsModules) { + this.compiler = compiler; + this.supportsEs6Modules = supportsEs6Modules; + this.supportsCommonJsModules = supportsCommonJsModules; + } + + public void process(Node root) { + checkArgument(root.isScript()); + if (FindModuleDependencies.isEs6ModuleRoot(root)) { + moduleType = ModuleType.ES6; + } + CompilerInput input = compiler.getInput(root.getInputId()); + + // The "goog" namespace isn't always specifically required. + // The deps parser will pick up any access to a `goog.foo()` call + // 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"); + } + + NodeTraversal.traverseEs6(compiler, root, this); + + if (moduleType == ModuleType.ES6) { + convertToEs6Module(root, true); + } + input.addProvide(input.getPath().toModuleName()); + input.setJsModuleType(moduleType); + input.setHasFullParseDependencyInfo(true); + } + + @Override + public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) { + return true; + } + + @Override + public void visit(NodeTraversal t, Node n, Node parent) { + if (supportsEs6Modules && n.isExport()) { + moduleType = ModuleType.ES6; + + } else if (supportsEs6Modules && n.isImport()) { + moduleType = ModuleType.ES6; + String moduleName; + String importName = n.getLastChild().getString(); + boolean isNamespaceImport = importName.startsWith("goog:"); + if (isNamespaceImport) { + // Allow importing Closure namespace objects (e.g. from goog.provide or goog.module) as + // import ... from 'goog:my.ns.Object'. + // These are rewritten to plain namespace object accesses. + moduleName = importName.substring("goog:".length()); + } else { + ModuleLoader.ModulePath modulePath = + t.getInput() + .getPath() + .resolveJsModule(importName, n.getSourceFileName(), n.getLineno(), n.getCharno()); + if (modulePath == null) { + // The module loader issues an error + // Fall back to assuming the module is a file path + modulePath = t.getInput().getPath().resolveModuleAsPath(importName); + } + moduleName = modulePath.toModuleName(); + } + if (moduleName.startsWith("goog.")) { + t.getInput().addOrderedRequire("goog"); + } + t.getInput().addOrderedRequire(moduleName); + } else if (supportsCommonJsModules) { + if (ProcessCommonJSModules.isCommonJsExport(t, n)) { + moduleType = ModuleType.COMMONJS; + } else if (ProcessCommonJSModules.isCommonJsImport(n)) { + String path = ProcessCommonJSModules.getCommonJsImportPath(n); + + ModuleLoader.ModulePath modulePath = + t.getInput() + .getPath() + .resolveJsModule(path, n.getSourceFileName(), n.getLineno(), n.getCharno()); + + if (modulePath != null) { + t.getInput().addOrderedRequire(modulePath.toModuleName()); + } + } + + // TODO(ChadKillingsworth) add require.ensure support + } + + if (parent != null + && (parent.isExprResult() || !t.inGlobalHoistScope()) + && n.isCall() + && n.getFirstChild().matchesQualifiedName("goog.require") + && n.getSecondChild() != null + && n.getSecondChild().isString()) { + String namespace = n.getSecondChild().getString(); + if (namespace.startsWith("goog.")) { + t.getInput().addOrderedRequire("goog"); + } + t.getInput().addOrderedRequire(namespace); + } + } + + /** Return whether or not the given script node represents an ES6 module file. */ + public static boolean isEs6ModuleRoot(Node scriptNode) { + checkArgument(scriptNode.isScript()); + if (scriptNode.getBooleanProp(Node.GOOG_MODULE)) { + return false; + } + return scriptNode.hasChildren() && scriptNode.getFirstChild().isModuleBody(); + } + + /** + * Convert a script into a module by marking it's root node as a module body. This allows a script + * which is imported as a module to be scoped as a module even without "import" or "export" + * statements. Fails if the file contains a goog.provide or goog.module. + * + * @return True, if the file is now an ES6 module. False, if the file must remain a script. + */ + public boolean convertToEs6Module(Node root) { + return this.convertToEs6Module(root, false); + } + + private boolean convertToEs6Module(Node root, boolean skipGoogProvideModuleCheck) { + if (isEs6ModuleRoot(root)) { + return true; + } + if (!skipGoogProvideModuleCheck) { + FindGoogProvideOrGoogModule finder = new FindGoogProvideOrGoogModule(); + NodeTraversal.traverseEs6(compiler, root, finder); + if (finder.isFound()) { + return false; + } + } + Node moduleNode = new Node(Token.MODULE_BODY).srcref(root); + moduleNode.addChildrenToBack(root.removeChildren()); + root.addChildToBack(moduleNode); + return true; + } +} diff --git a/src/com/google/javascript/jscomp/JSModuleGraph.java b/src/com/google/javascript/jscomp/JSModuleGraph.java index c249c7e0dca..ed97a4a409b 100644 --- a/src/com/google/javascript/jscomp/JSModuleGraph.java +++ b/src/com/google/javascript/jscomp/JSModuleGraph.java @@ -40,6 +40,7 @@ import java.util.BitSet; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.LinkedHashSet; @@ -465,11 +466,33 @@ public List manageDependencies( // Figure out which sources *must* be in each module, or in one // of that module's dependencies. + List orderedInputs = new ArrayList<>(); + Set reachedInputs = new HashSet<>(); for (JSModule module : entryPointInputsPerModule.keySet()) { - List transitiveClosure = - sorter.getDependenciesOf( - entryPointInputsPerModule.get(module), - depOptions.shouldSortDependencies()); + List transitiveClosure; + // Prefer a depth first ordering of dependencies from entry points. + // Always orders in a deterministic fashion regardless of the order of provided inputs + // given the same entry points in the same order. + if (depOptions.shouldSortDependencies() && depOptions.shouldPruneDependencies()) { + transitiveClosure = new ArrayList<>(); + // We need the ful set of dependencies for each module, so start with the full input set + Set inputsNotYetReached = new HashSet<>(inputs); + for (CompilerInput entryPoint : entryPointInputsPerModule.get(module)) { + transitiveClosure.addAll(getDepthFirstDependenciesOf(entryPoint, inputsNotYetReached)); + } + // For any input we have not yet reached, add them to the ordered list + for (CompilerInput orderedInput : transitiveClosure) { + if (reachedInputs.add(orderedInput)) { + orderedInputs.add(orderedInput); + } + } + } else { + // Simply order inputs so that any required namespace comes before it's usage. + // Ordered result varies based on the original order of inputs. + transitiveClosure = + sorter.getDependenciesOf( + entryPointInputsPerModule.get(module), depOptions.shouldSortDependencies()); + } for (CompilerInput input : transitiveClosure) { JSModule oldModule = input.getModule(); if (oldModule == null) { @@ -481,10 +504,14 @@ public List manageDependencies( } } } + if (!(depOptions.shouldSortDependencies() && depOptions.shouldPruneDependencies()) + || entryPointInputsPerModule.isEmpty()) { + orderedInputs = absoluteOrder; + } // All the inputs are pointing to the modules that own them. Yeah! // Update the modules to reflect this. - for (CompilerInput input : absoluteOrder) { + for (CompilerInput input : orderedInputs) { JSModule module = input.getModule(); if (module != null) { module.add(input); @@ -500,6 +527,42 @@ public List manageDependencies( return result.build(); } + /** + * Given an input and set of unprocessed inputs, return the input and it's dependencies by + * performing a recursive, depth-first traversal. + */ + private List getDepthFirstDependenciesOf( + CompilerInput rootInput, Set unreachedInputs) { + List orderedInputs = new ArrayList<>(); + if (!unreachedInputs.remove(rootInput)) { + return orderedInputs; + } + + for (String importedNamespace : rootInput.getRequires()) { + CompilerInput dependency = + JSModuleGraph.findInputProviding(importedNamespace, unreachedInputs); + if (dependency != null) { + orderedInputs.addAll(getDepthFirstDependenciesOf(dependency, unreachedInputs)); + } + } + + orderedInputs.add(rootInput); + return orderedInputs; + } + + private static CompilerInput findInputProviding(String namespace, Set inputs) { + for (CompilerInput input : inputs) { + if (namespace.startsWith("module$")) { + if (input.getPath().toModuleName().equals(namespace)) { + return input; + } + } else if (input.getProvides().contains(namespace)) { + return input; + } + } + return null; + } + private Collection createEntryPointInputs( DependencyOptions depOptions, List inputs, @@ -507,8 +570,14 @@ private Collection createEntryPointInputs( throws MissingModuleException, MissingProvideException { Set entryPointInputs = new LinkedHashSet<>(); Map modulesByName = getModulesByName(); - if (depOptions.shouldPruneDependencies()) { + // Some files implicitly depend on base.js without actually requiring anything. + // So we always treat it as the first entry point to ensure it's ordered correctly. + CompilerInput baseJs = sorter.maybeGetInputProviding("goog"); + if (baseJs != null) { + entryPointInputs.add(baseJs); + } + if (!depOptions.shouldDropMoochers()) { entryPointInputs.addAll(sorter.getInputsWithoutProvides()); } @@ -538,11 +607,6 @@ private Collection createEntryPointInputs( entryPointInputs.add(entryPointInput); } - - CompilerInput baseJs = sorter.maybeGetInputProviding("goog"); - if (baseJs != null) { - entryPointInputs.add(baseJs); - } } else { entryPointInputs.addAll(inputs); } diff --git a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java index e86bf6e74f2..bda4322407e 100644 --- a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java +++ b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java @@ -28,9 +28,7 @@ import com.google.javascript.rhino.JSDocInfoBuilder; import com.google.javascript.rhino.Node; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; /** * Rewrites a CommonJS module http://wiki.commonjs.org/wiki/Modules/1.1.1 @@ -44,6 +42,7 @@ public final class ProcessCommonJSModules implements CompilerPass { private static final String EXPORTS = "exports"; private static final String MODULE = "module"; + private static final String REQUIRE = "require"; public static final DiagnosticType UNKNOWN_REQUIRE_ENSURE = DiagnosticType.warning( @@ -56,7 +55,6 @@ public final class ProcessCommonJSModules implements CompilerPass { + " Did you actually intend to export something?"); private final Compiler compiler; - private final boolean reportDependencies; /** * Creates a new ProcessCommonJSModules instance which can be used to @@ -65,23 +63,7 @@ public final class ProcessCommonJSModules implements CompilerPass { * @param compiler The compiler */ public ProcessCommonJSModules(Compiler compiler) { - this(compiler, true); - } - - /** - * Creates a new ProcessCommonJSModules instance which can be used to - * rewrite CommonJS modules to a concatenable form. - * - * @param compiler The compiler - * @param reportDependencies Whether the rewriter should report dependency - * information to the Closure dependency manager. This needs to be true - * if we want to sort CommonJS module inputs correctly. Note that goog.provide - * and goog.require calls will still be generated if this argument is - * false. - */ - public ProcessCommonJSModules(Compiler compiler, boolean reportDependencies) { this.compiler = compiler; - this.reportDependencies = reportDependencies; } @@ -92,12 +74,16 @@ public ProcessCommonJSModules(Compiler compiler, boolean reportDependencies) { */ @Override public void process(Node externs, Node root) { + process(externs, root, false); + } + + public void process(Node externs, Node root, boolean forceModuleDetection) { checkState(root.isScript()); FindImportsAndExports finder = new FindImportsAndExports(); NodeTraversal.traverseEs6(compiler, root, finder); ImmutableList.Builder exports = ImmutableList.builder(); - if (finder.isCommonJsModule()) { + if (finder.isCommonJsModule() || forceModuleDetection) { finder.reportModuleErrors(); if (!finder.umdPatterns.isEmpty()) { @@ -127,9 +113,55 @@ public void process(Node externs, Node root) { } NodeTraversal.traverseEs6( - compiler, root, new RewriteModule(finder.isCommonJsModule(), exports.build())); + compiler, + root, + new RewriteModule(finder.isCommonJsModule() || forceModuleDetection, exports.build())); + } - finder.addGoogProvideAndRequires(); + /** + * Recognize if a node is a module import. We recognize two forms: + * + * - require("something"); + * - __webpack_require__(4); // only when the module resolution is WEBPACK + */ + public static boolean isCommonJsImport(Node requireCall) { + if (requireCall.isCall() && requireCall.hasTwoChildren()) { + if (requireCall.getFirstChild().matchesQualifiedName(REQUIRE) + && requireCall.getSecondChild().isString()) { + return true; + } + } + return false; + } + + public static String getCommonJsImportPath(Node requireCall) { + return requireCall.getSecondChild().getString(); + } + + /** + * Recognize if a node is a module export. We recognize several forms: + * + * - module.exports = something; + * - module.exports.something = something; + * - exports.something = something; + * - __webpack_exports__["something"] = something; // only when the module resolution is WEBPACK + * + *

In addition, we only recognize an export if the base export object is not defined or is + * defined in externs. + */ + public static boolean isCommonJsExport(NodeTraversal t, Node export) { + if (export.matchesQualifiedName(MODULE + "." + EXPORTS)) { + Var v = t.getScope().getVar(MODULE); + if (v == null || v.isExtern()) { + return true; + } + } else if (export.isName() && EXPORTS.equals(export.getString())) { + Var v = t.getScope().getVar(export.getString()); + if (v == null || v.isGlobal()) { + return true; + } + } + return false; } /** @@ -240,13 +272,13 @@ class FindImportsAndExports implements NodeTraversal.Callback { private Node script = null; boolean isCommonJsModule() { - return (exports.size() > 0 || moduleExports.size() > 0) && !hasGoogProvideOrModule; + return (exports.size() > 0 || moduleExports.size() > 0) + && !hasGoogProvideOrModule; } List umdPatterns = new ArrayList<>(); List moduleExports = new ArrayList<>(); List exports = new ArrayList<>(); - Set imports = new HashSet<>(); List errors = new ArrayList<>(); public List getModuleExports() { @@ -289,12 +321,8 @@ public void visit(NodeTraversal t, Node n, Node parent) { visitRequireEnsureCall(t, n); } - if (n.matchesQualifiedName("module.exports")) { - Var v = t.getScope().getVar(MODULE); - // only rewrite "module.exports" if "module" is a free variable, - // meaning it is not defined in the current scope as a local - // variable or function parameter - if (v == null) { + if (n.matchesQualifiedName(MODULE + "." + EXPORTS)) { + if (ProcessCommonJSModules.isCommonJsExport(t, n)) { moduleExports.add(new ExportInfo(n, t.getScope())); // If the module.exports statement is nested in the then branch of an if statement, @@ -343,17 +371,14 @@ public void visit(NodeTraversal t, Node n, Node parent) { } } - if (n.isCall() - && n.hasTwoChildren() - && n.getFirstChild().matchesQualifiedName("require") - && n.getSecondChild().isString()) { + if (ProcessCommonJSModules.isCommonJsImport(n)) { visitRequireCall(t, n, parent); } } - /** Visit require calls. Emit corresponding goog.require call. */ + /** Visit require calls. */ private void visitRequireCall(NodeTraversal t, Node require, Node parent) { - String requireName = require.getSecondChild().getString(); + String requireName = ProcessCommonJSModules.getCommonJsImportPath(require); ModulePath modulePath = t.getInput() .getPath() @@ -367,20 +392,15 @@ private void visitRequireCall(NodeTraversal t, Node require, Node parent) { return; } - - String moduleName = modulePath.toModuleName(); - // When require("name") is used as a standalone statement (the result isn't used) // it indicates that a module is being loaded for the side effects it produces. - // In this case the require statement should just be removed as the goog.require - // call inserted will import the module. + // In this case the require statement should just be removed as the dependency + // sorting will insert the file for us. if (!NodeUtil.isExpressionResultUsed(require) && parent.isExprResult() && NodeUtil.isStatementBlock(parent.getParent())) { parent.detach(); } - - imports.add(moduleName); } /** @@ -467,13 +487,6 @@ void initializeModule() { String moduleName = modulePath.toModuleName(); - // The default declaration for the goog.provide is a constant so - // we need to declare the variable if we have more than one - // assignment to module.exports or those assignments are not - // at the top level. - // - // If we assign to the variable more than once or all the assignments - // are properties, initialize the variable as well. int directAssignmentsAtTopLevel = 0; int directAssignments = 0; for (ExportInfo export : moduleExports) { @@ -517,48 +530,6 @@ void initializeModule() { } } - /** - * Add goog.require statements for any require statements and a goog.provide statement for the - * module - */ - void addGoogProvideAndRequires() { - CompilerInput ci = compiler.getInput(this.script.getInputId()); - ModulePath modulePath = ci.getPath(); - if (modulePath == null) { - return; - } - - String moduleName = modulePath.toModuleName(); - - for (String importName : imports) { - // Add goog.provide calls. - if (reportDependencies) { - ci.addRequire(importName); - } - - this.script.addChildToFront( - IR.exprResult( - IR.call( - IR.getprop(IR.name("goog"), IR.string("require")), IR.string(importName))) - .useSourceInfoIfMissingFromForTree(this.script)); - } - - if (isCommonJsModule()) { - // Add goog.provide calls. - if (reportDependencies) { - ci.addProvide(moduleName); - } - this.script.addChildToFront( - IR.exprResult( - IR.call( - IR.getprop(IR.name("goog"), IR.string("provide")), IR.string(moduleName))) - .useSourceInfoIfMissingFromForTree(this.script)); - compiler.reportChangeToEnclosingScope(this.script); - } else if (imports.size() > 0) { - compiler.reportChangeToEnclosingScope(this.script); - } - } - /** Find the outermost if node ancestor for a node without leaving the function scope */ private Node getOutermostIfAncestor(Node n) { if (n == null || NodeUtil.isTopLevel(n) || n.isFunction()) { @@ -687,9 +658,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { break; case CALL: - if (n.hasTwoChildren() - && n.getFirstChild().matchesQualifiedName("require") - && n.getSecondChild().isString()) { + if (ProcessCommonJSModules.isCommonJsImport(n)) { imports.add(n); } break; @@ -768,7 +737,8 @@ public void visit(NodeTraversal t, Node n, Node parent) { * module. By this point all references to the import alias should have already been renamed. */ private void visitRequireCall(NodeTraversal t, Node require, Node parent) { - String requireName = require.getSecondChild().getString(); + String requireName = ProcessCommonJSModules.getCommonJsImportPath(require); + ModulePath modulePath = t.getInput() .getPath() @@ -1287,11 +1257,9 @@ private String getModuleImportName(NodeTraversal t, Node n) { if (rValue.isCall()) { // var foo = require('bar'); - if (rValue.hasTwoChildren() - && rValue.getFirstChild().matchesQualifiedName("require") - && rValue.getSecondChild().isString() + if (ProcessCommonJSModules.isCommonJsImport(rValue) && t.getScope().getVar(rValue.getFirstChild().getQualifiedName()) == null) { - String requireName = rValue.getSecondChild().getString(); + String requireName = ProcessCommonJSModules.getCommonJsImportPath(rValue); ModulePath modulePath = t.getInput() .getPath() diff --git a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java index 8dbb70b7bfd..a191f80d6af 100644 --- a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java +++ b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java @@ -736,6 +736,7 @@ public void testSourceSortingOn2() { test(new String[] { "goog.provide('a');", "goog.require('a');\n" + + "/** This is base.js */\n" + "var COMPILED = false;", }, new String[] { @@ -747,14 +748,12 @@ public void testSourceSortingOn2() { public void testSourceSortingOn3() { args.add("--dependency_mode=LOOSE"); args.add("--language_in=ECMASCRIPT5"); - test(new String[] { + test( + new String[] { "goog.addDependency('sym', [], []);\nvar x = 3;", - "var COMPILED = false;", - }, - new String[] { - "var COMPILED = !1;", - "var x = 3;" - }); + "/** This is base.js */\nvar COMPILED = false;", + }, + new String[] {"var COMPILED = !1;", "var x = 3;"}); } public void testSourceSortingCircularDeps1() { @@ -823,15 +822,13 @@ public void testSourcePruningOn3() { public void testSourcePruningOn4() { args.add("--entry_point=goog:scotch"); args.add("--entry_point=goog:beer"); - test(new String[] { + test( + new String[] { "goog.provide('guinness');\ngoog.require('beer');", "goog.provide('beer');", "goog.provide('scotch'); var x = 3;" - }, - new String[] { - "var beer = {};", - "var scotch = {}, x = 3;", - }); + }, + new String[] {"var scotch = {}, x = 3;", "var beer = {};"}); } public void testSourcePruningOn5() { @@ -854,18 +851,20 @@ public void testSourcePruningOn6() { new String[] { "var beer = {};", "", - "var scotch = {}, x = 3;", + "var scotch = {}, x = 3;" }); + assertTrue(lastCompiler.getOptions().getDependencyOptions().shouldSortDependencies()); + assertTrue(lastCompiler.getOptions().getDependencyOptions().shouldPruneDependencies()); } public void testSourcePruningOn7() { args.add("--dependency_mode=LOOSE"); test(new String[] { - "var COMPILED = false;", - }, - new String[] { + "/** This is base.js */\nvar COMPILED = false;", + }, + new String[] { "var COMPILED = !1;", - }); + }); } public void testSourcePruningOn8() { @@ -1826,8 +1825,7 @@ public void testES6ImportOfCJS() { "/** @constructor */ var module$foo = function(){};", "module$foo.prototype.bar=function(){console.log(\"bar\")};"), LINE_JOINER.join( - "var module$app = {}, baz$$module$app = new module$foo();", - "console.log(baz$$module$app.bar());") + "var baz$$module$app = new module$foo();", "console.log(baz$$module$app.bar());") }); } @@ -1844,10 +1842,8 @@ public void testES6ImportOfFileWithoutImportsOrExports() { }, new String[] { CompilerTestCase.LINE_JOINER.join( - "/** @const */ var module$foo={};", - "function foo$$module$foo(){ alert('foo'); }", - "foo$$module$foo();"), - "var module$app = {};" + "function foo$$module$foo(){ alert('foo'); }", "foo$$module$foo();"), + "" }); } @@ -1870,10 +1866,8 @@ public void testES6ImportOfFileWithImportsButNoExports() { " $jscompDefaultExport$$module$message = 'message';", "module$message.default = $jscompDefaultExport$$module$message;"), CompilerTestCase.LINE_JOINER.join( - "/** @const */ var module$foo={};", - "function foo$$module$foo(){ alert(module$message.default); }", - "foo$$module$foo();"), - "var module$app = {};" + "function foo$$module$foo(){ alert(module$message.default); }", "foo$$module$foo();"), + "" }); } @@ -1892,7 +1886,6 @@ public void testCommonJSRequireOfFileWithoutExports() { }, new String[] { CompilerTestCase.LINE_JOINER.join( - "/** @const */ var module$foo={};", "function foo$$module$foo(){ alert('foo'); }", "foo$$module$foo();"), CompilerTestCase.LINE_JOINER.join("'use strict';", "") diff --git a/test/com/google/javascript/jscomp/CompilerTest.java b/test/com/google/javascript/jscomp/CompilerTest.java index 4e96a2a0d9d..f025f9a7049 100644 --- a/test/com/google/javascript/jscomp/CompilerTest.java +++ b/test/com/google/javascript/jscomp/CompilerTest.java @@ -1468,4 +1468,107 @@ public CompilerInput getCachedCompilerInput(SourceFile source) { compiler.init(ImmutableList.of(), sources, options); assertThat(compiler.getModules().get(0).getInputs()).contains(input); } + + public void testProperEs6ModuleOrdering() throws Exception { + List sources = new ArrayList<>(); + sources.add(SourceFile.fromCode( + "/entry.js", + CompilerTestCase.LINE_JOINER.join( + "import './b/b.js';", + "import './b/a.js';", + "import './important.js';", + "import './a/b.js';", + "import './a/a.js';"))); + sources.add(SourceFile.fromCode("/a/a.js", "window['D'] = true;")); + sources.add(SourceFile.fromCode("/a/b.js", "window['C'] = true;")); + sources.add(SourceFile.fromCode("/b/a.js", "window['B'] = true;")); + sources.add(SourceFile.fromCode( + "/b/b.js", + CompilerTestCase.LINE_JOINER.join( + "import foo from './c.js';", + "if (foo.settings.inUse) {", + " window['E'] = true;", + "}", + "window['A'] = true;"))); + sources.add(SourceFile.fromCode( + "/b/c.js", + CompilerTestCase.LINE_JOINER.join( + "window['BEFOREA'] = true;", + "", + "export default {", + " settings: {", + " inUse: Boolean(document.documentElement['attachShadow'])", + " }", + "};"))); + sources.add(SourceFile.fromCode("/important.js", "window['E'] = false;")); + + CompilerOptions options = new CompilerOptions(); + options.setLanguageIn(LanguageMode.ECMASCRIPT_2015); + options.setLanguageOut(LanguageMode.ECMASCRIPT5); + options.dependencyOptions.setEntryPoints(ImmutableList.of(ModuleIdentifier.forFile("/entry.js"))); + options.dependencyOptions.setDependencySorting(true); + options.dependencyOptions.setDependencyPruning(true); + options.dependencyOptions.setMoocherDropping(true); + List externs = + AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment()); + Compiler compiler = new Compiler(); + Result result = compiler.compile(externs, ImmutableList.copyOf(sources), options); + assertTrue(result.success); + + List orderedInputs = new ArrayList<>(); + for (CompilerInput input : compiler.getInputsInOrder()) { + orderedInputs.add(input.getName()); + } + + assertThat(orderedInputs) + .containsExactly("/b/c.js", "/b/b.js", "/b/a.js", "/important.js", "/a/b.js", "/a/a.js", "/entry.js") + .inOrder(); + } + + public void testProperGoogBaseOrdering() throws Exception { + List sources = new ArrayList<>(); + sources.add(SourceFile.fromCode("test.js", "goog.setTestOnly()")); + sources.add(SourceFile.fromCode("d.js", "goog.provide('d');")); + sources.add(SourceFile.fromCode("c.js", "goog.provide('c');")); + sources.add(SourceFile.fromCode("b.js", "goog.provide('b');")); + sources.add(SourceFile.fromCode("a.js", "goog.provide('a');")); + sources.add(SourceFile.fromCode("base.js", + CompilerTestCase.LINE_JOINER.join( + "/** @provideGoog */", + "/** @const */ var goog = goog || {};", + "var COMPILED = false;"))); + sources.add(SourceFile.fromCode("entry.js", + CompilerTestCase.LINE_JOINER.join( + "goog.require('a');", + "goog.require('b');", + "goog.require('c');", + "goog.require('d');"))); + + CompilerOptions options = new CompilerOptions(); + options.setLanguageIn(LanguageMode.ECMASCRIPT_2015); + options.setLanguageOut(LanguageMode.ECMASCRIPT5); + options.dependencyOptions.setEntryPoints(ImmutableList.of(ModuleIdentifier.forFile("entry.js"))); + options.dependencyOptions.setDependencySorting(true); + options.dependencyOptions.setDependencyPruning(true); + options.dependencyOptions.setMoocherDropping(false); + List externs = + AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment()); + + for (int iterationCount = 0; iterationCount < 10; iterationCount++) { + java.util.Collections.shuffle(sources); + Compiler compiler = new Compiler(); + Result result = compiler.compile(externs, ImmutableList.copyOf(sources), options); + assertTrue(result.success); + + List orderedInputs = new ArrayList<>(); + for (CompilerInput input : compiler.getInputsInOrder()) { + orderedInputs.add(input.getName()); + } + + assertThat(orderedInputs) + .containsExactly("base.js", "test.js", "a.js", "b.js", "c.js", "d.js", "entry.js"); + assertThat(orderedInputs.indexOf("base.js")).isLessThan(orderedInputs.indexOf("entry.js")); + assertThat(orderedInputs.indexOf("base.js")).isLessThan(orderedInputs.indexOf("test.js")); + } + } } diff --git a/test/com/google/javascript/jscomp/Es6RewriteModulesTest.java b/test/com/google/javascript/jscomp/Es6RewriteModulesTest.java index 6be91572a29..750482462aa 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteModulesTest.java +++ b/test/com/google/javascript/jscomp/Es6RewriteModulesTest.java @@ -62,36 +62,23 @@ protected int getNumRepetitions() { } void testModules(String input, String expected) { - ModulesTestUtils.testModules( - this, - "testcode.js", - input, - LINE_JOINER.join( - "/** @fileoverview", - " * @suppress {missingProvide|missingRequire}", - " */", - "goog.provide('module$testcode');", - expected)); + ModulesTestUtils.testModules(this, "testcode.js", input, expected); } public void testImport() { - testModules( - "import name from './other.js';\n use(name);", - "goog.require('module$other'); use(module$other.default);"); + testModules("import name from './other.js';\n use(name);", "use(module$other.default);"); - testModules("import {n as name} from './other.js';", "goog.require('module$other');"); + testModules("import {n as name} from './other.js';", ""); testModules( "import x, {f as foo, b as bar} from './other.js';\n use(x);", - "goog.require('module$other'); use(module$other.default);"); + "use(module$other.default);"); testModules( - "import {default as name} from './other.js';\n use(name);", - "goog.require('module$other'); use(module$other.default);"); + "import {default as name} from './other.js';\n use(name);", "use(module$other.default);"); testModules( - "import {class as name} from './other.js';\n use(name);", - "goog.require('module$other'); use(module$other.class);"); + "import {class as name} from './other.js';\n use(name);", "use(module$other.class);"); } public void testImport_missing() { @@ -100,22 +87,20 @@ public void testImport_missing() { } public void testImportStar() { - testModules( - "import * as name from './other.js';\n use(name.foo);", - "goog.require('module$other');\n use(module$other.foo)"); + testModules("import * as name from './other.js';\n use(name.foo);", "use(module$other.foo)"); } public void testTypeNodeRewriting() { testModules( "import * as name from './other.js';\n /** @type {name.foo} */ var x;", - "goog.require('module$other');" - + "/** @type {module$other.foo} */ var x$$module$testcode;"); + "/** @type {module$other.foo} */ var x$$module$testcode;"); } public void testExport() { testModules( "export var a = 1, b = 2;", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var a$$module$testcode = 1, b$$module$testcode = 2;", "module$testcode.a = a$$module$testcode;", "module$testcode.b = b$$module$testcode;")); @@ -123,6 +108,7 @@ public void testExport() { testModules( "export var a;\nexport var b;", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var a$$module$testcode; var b$$module$testcode;", "module$testcode.a = a$$module$testcode;", "module$testcode.b = b$$module$testcode;")); @@ -130,12 +116,14 @@ public void testExport() { testModules( "export function f() {};", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "function f$$module$testcode() {}", "module$testcode.f = f$$module$testcode;")); testModules( "export function f() {};\nfunction g() { f(); }", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "function f$$module$testcode() {}", "function g$$module$testcode() { f$$module$testcode(); }", "module$testcode.f = f$$module$testcode;")); @@ -143,6 +131,7 @@ public void testExport() { testModules( LINE_JOINER.join("export function MyClass() {};", "MyClass.prototype.foo = function() {};"), LINE_JOINER.join( + "/** @const */ var module$testcode={};", "function MyClass$$module$testcode() {}", "MyClass$$module$testcode.prototype.foo = function() {};", "module$testcode.MyClass = MyClass$$module$testcode;")); @@ -150,6 +139,7 @@ public void testExport() { testModules( "var f = 1;\nvar b = 2;\nexport {f as foo, b as bar};", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var f$$module$testcode = 1;", "var b$$module$testcode = 2;", "module$testcode.foo = f$$module$testcode;", @@ -158,12 +148,14 @@ public void testExport() { testModules( "var f = 1;\nexport {f as default};", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var f$$module$testcode = 1;", "module$testcode.default = f$$module$testcode;")); testModules( "var f = 1;\nexport {f as class};", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var f$$module$testcode = 1;", "module$testcode.class = f$$module$testcode;")); } @@ -172,6 +164,7 @@ public void testExportWithJsDoc() { testModules( "/** @constructor */\nexport function F() { return '';}", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "/** @constructor */", "function F$$module$testcode() { return ''; }", "module$testcode.F = F$$module$testcode")); @@ -179,6 +172,7 @@ public void testExportWithJsDoc() { testModules( "/** @return {string} */\nexport function f() { return '';}", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "/** @return {string} */", "function f$$module$testcode() { return ''; }", "module$testcode.f = f$$module$testcode")); @@ -186,6 +180,7 @@ public void testExportWithJsDoc() { testModules( "/** @return {string} */\nexport var f = function() { return '';}", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "/** @return {string} */", "var f$$module$testcode = function() { return ''; }", "module$testcode.f = f$$module$testcode")); @@ -193,6 +188,7 @@ public void testExportWithJsDoc() { testModules( "/** @type {number} */\nexport var x = 3", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "/** @type {number} */", "var x$$module$testcode = 3;", "module$testcode.x = x$$module$testcode")); @@ -202,7 +198,7 @@ public void testImportAndExport() { testModules( LINE_JOINER.join("import {name as n} from './other.js';", "use(n);", "export {n as name};"), LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "use(module$other.name);", "module$testcode.name = module$other.name;")); } @@ -214,7 +210,7 @@ public void testExportFrom() { "export {default} from './other.js';", "export {class} from './other.js';"), LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "module$testcode.name = module$other.name;", "module$testcode.default = module$other.default;", "module$testcode.class = module$other.class;")); @@ -222,7 +218,7 @@ public void testExportFrom() { testModules( "export {a, b as c, d} from './other.js';", LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "module$testcode.a = module$other.a;", "module$testcode.c = module$other.b;", "module$testcode.d = module$other.d;")); @@ -230,7 +226,7 @@ public void testExportFrom() { testModules( "export {a as b, b as a} from './other.js';", LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "module$testcode.b = module$other.a;", "module$testcode.a = module$other.b;")); @@ -240,7 +236,7 @@ public void testExportFrom() { "export {a as a2, default as b} from './other.js';", "export {class as switch} from './other.js';"), LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "module$testcode.a = module$other.default;", "module$testcode.a2 = module$other.a;", "module$testcode.b = module$other.default;", @@ -251,12 +247,14 @@ public void testExportDefault() { testModules( "export default 'someString';", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var $jscompDefaultExport$$module$testcode = 'someString';", "module$testcode.default = $jscompDefaultExport$$module$testcode;")); testModules( "var x = 5;\nexport default x;", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var x$$module$testcode = 5;", "var $jscompDefaultExport$$module$testcode = x$$module$testcode;", "module$testcode.default = $jscompDefaultExport$$module$testcode;")); @@ -264,6 +262,7 @@ public void testExportDefault() { testModules( "export default function f(){};\n var x = f();", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "function f$$module$testcode() {}", "var x$$module$testcode = f$$module$testcode();", "module$testcode.default = f$$module$testcode;")); @@ -271,6 +270,7 @@ public void testExportDefault() { testModules( "export default class Foo {};\n var x = new Foo;", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "class Foo$$module$testcode {}", "var x$$module$testcode = new Foo$$module$testcode;", "module$testcode.default = Foo$$module$testcode;")); @@ -280,12 +280,14 @@ public void testExportDefault_anonymous() { testModules( "export default class {};", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var $jscompDefaultExport$$module$testcode = class {};", "module$testcode.default = $jscompDefaultExport$$module$testcode;")); testModules( "export default function() {}", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var $jscompDefaultExport$$module$testcode = function() {}", "module$testcode.default = $jscompDefaultExport$$module$testcode;")); } @@ -299,7 +301,6 @@ public void testExtendImportedClass() { " useParent(parent) {}", "}"), LINE_JOINER.join( - "goog.require('module$other');", "class Child$$module$testcode extends module$other.Parent {", " /** @param {Parent$$module$other} parent */", " useParent(parent) {}", @@ -313,7 +314,7 @@ public void testExtendImportedClass() { " useParent(parent) {}", "}"), LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "class Child$$module$testcode extends module$other.Parent {", " /** @param {Parent$$module$other} parent */", " useParent(parent) {}", @@ -326,6 +327,7 @@ public void testFixTypeNode() { LINE_JOINER.join( "export class Child {", " /** @param {Child} child */", " useChild(child) {}", "}"), LINE_JOINER.join( + "/** @const */ var module$testcode={};", "class Child$$module$testcode {", " /** @param {Child$$module$testcode} child */", " useChild(child) {}", @@ -339,6 +341,7 @@ public void testFixTypeNode() { " useBaz(baz) {}", "}"), LINE_JOINER.join( + "/** @const */ var module$testcode={};", "class Child$$module$testcode {", " /** @param {Child$$module$testcode.Foo.Bar.Baz} baz */", " useBaz(baz) {}", @@ -352,6 +355,7 @@ public void testReferenceToTypeFromOtherModule() { LINE_JOINER.join( "export class Foo {", " /** @param {./other.Baz} baz */", " useBaz(baz) {}", "}"), LINE_JOINER.join( + "/** @const */ var module$testcode={};", "class Foo$$module$testcode {", " /** @param {module$other.Baz} baz */", " useBaz(baz) {}", @@ -360,11 +364,9 @@ public void testReferenceToTypeFromOtherModule() { testModules( LINE_JOINER.join( - "export class Foo {", - " /** @param {/other.Baz} baz */", - " useBaz(baz) {}", - "}"), + "export class Foo {", " /** @param {/other.Baz} baz */", " useBaz(baz) {}", "}"), LINE_JOINER.join( + "/** @const */ var module$testcode={};", "class Foo$$module$testcode {", " /** @param {module$other.Baz} baz */", " useBaz(baz) {}", @@ -379,7 +381,6 @@ public void testReferenceToTypeFromOtherModule() { " useParent(parent) {}", "}"), LINE_JOINER.join( - "goog.require('module$other');", "class Child$$module$testcode extends module$other.Parent {", " /** @param {module$other.Parent} parent */", " useParent(parent) {}", @@ -392,7 +393,7 @@ public void testRenameTypedef() { LINE_JOINER.join( "import './other.js';", "/** @typedef {string|!Object} */", "export var UnionType;"), LINE_JOINER.join( - "goog.require('module$other');", + "/** @const */ var module$testcode={};", "/** @typedef {string|!Object} */", "var UnionType$$module$testcode;", "/** @typedef {UnionType$$module$testcode} */", @@ -409,6 +410,7 @@ public void testNoInnerChange() { "}());", "export { Foo };"), LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var Foo$$module$testcode = function() {", " /** @param bar */", " function Foo(bar) {}", @@ -432,7 +434,6 @@ public void testRenameImportedReference() { " }", "}"), LINE_JOINER.join( - "goog.require('module$other');", "module$other.f();", "function g$$module$testcode() {", " module$other.f();", @@ -451,6 +452,7 @@ public void testGoogRequires_noChange() { testModules( "goog.require('foo.bar');\nexport var x;", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "goog.require('foo.bar');", "var x$$module$testcode;", "module$testcode.x = x$$module$testcode")); @@ -458,23 +460,23 @@ public void testGoogRequires_noChange() { testModules( "export var x;\n goog.require('foo.bar');", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var x$$module$testcode;", "goog.require('foo.bar');", "module$testcode.x = x$$module$testcode")); testModules( - "import * as s from './other.js';\ngoog.require('foo.bar');", - "goog.require('module$other'); goog.require('foo.bar');"); + "import * as s from './other.js';\ngoog.require('foo.bar');", "goog.require('foo.bar');"); testModules( - "goog.require('foo.bar');\nimport * as s from './other.js';", - "goog.require('module$other'); goog.require('foo.bar'); "); + "goog.require('foo.bar');\nimport * as s from './other.js';", "goog.require('foo.bar'); "); } public void testGoogRequires_rewrite() { testModules( "const bar = goog.require('foo.bar')\nexport var x;", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "goog.require('foo.bar');", "const bar$$module$testcode = foo.bar;", "var x$$module$testcode;", @@ -483,6 +485,7 @@ public void testGoogRequires_rewrite() { testModules( "export var x\nconst bar = goog.require('foo.bar');", LINE_JOINER.join( + "/** @const */ var module$testcode={};", "var x$$module$testcode;", "goog.require('foo.bar');", "const bar$$module$testcode = foo.bar;", @@ -491,14 +494,12 @@ public void testGoogRequires_rewrite() { testModules( "import * as s from './other.js';\nconst bar = goog.require('foo.bar');", LINE_JOINER.join( - "goog.require('module$other');", "goog.require('foo.bar');", "const bar$$module$testcode = foo.bar;")); testModules( "const bar = goog.require('foo.bar');\nimport * as s from './other.js';", LINE_JOINER.join( - "goog.require('module$other');", "goog.require('foo.bar');", "const bar$$module$testcode = foo.bar;")); } @@ -526,7 +527,6 @@ public void testGoogRequiresDestructuring_rewrite() { "const {foo, bar} = goog.require('some.name.space');", "use(foo, bar);"), LINE_JOINER.join( - "goog.require('module$other');", "goog.require('some.name.space');", "const {", " foo: foo$$module$testcode,", @@ -547,28 +547,17 @@ public void testGoogRequiresDestructuring_rewrite() { public void testNamespaceImports() { testModules( - LINE_JOINER.join( - "import Foo from 'goog:other.Foo';", - "use(Foo);"), - LINE_JOINER.join( - "goog.require('other.Foo');", - "use(other.Foo)")); + LINE_JOINER.join("import Foo from 'goog:other.Foo';", "use(Foo);"), "use(other.Foo)"); testModules( - LINE_JOINER.join( - "import {x, y} from 'goog:other.Foo';", - "use(x);", - "use(y);"), - LINE_JOINER.join( - "goog.require('other.Foo');", - "use(other.Foo.x);\n use(other.Foo.y);")); + LINE_JOINER.join("import {x, y} from 'goog:other.Foo';", "use(x);", "use(y);"), + "use(other.Foo.x);\n use(other.Foo.y);"); testModules( LINE_JOINER.join( "import Foo from 'goog:other.Foo';", "/** @type {Foo} */ var foo = new Foo();"), LINE_JOINER.join( - "goog.require('other.Foo');", "/** @type {other.Foo} */", "var foo$$module$testcode = new other.Foo();")); @@ -584,7 +573,6 @@ public void testObjectDestructuringAndObjLitShorthand() { "const {a, b} = f({foo});", "use(a, b);"), LINE_JOINER.join( - "goog.require('module$other');", "const foo$$module$testcode = 1;", "const {", " a: a$$module$testcode,", @@ -594,12 +582,10 @@ public void testObjectDestructuringAndObjLitShorthand() { } public void testImportWithoutReferences() { - testModules("import './other.js';", "goog.require('module$other');"); + testModules("import './other.js';", ""); // GitHub issue #1819: https://github.com/google/closure-compiler/issues/1819 // Need to make sure the order of the goog.requires matches the order of the imports. - testModules( - "import './other.js';\nimport './yet_another.js';", - "goog.require('module$other'); goog.require('module$yet_another');"); + testModules("import './other.js';\nimport './yet_another.js';", ""); } public void testUselessUseStrict() { @@ -623,41 +609,25 @@ public void testAbsoluteImportsWithModuleRoots() { Compiler.joinPathParts("base", "test", "sub.js"), "import * as foo from '/mod/name.js';")), ImmutableList.of( - SourceFile.fromCode( - Compiler.joinPathParts("base", "mod", "name.js"), - LINE_JOINER.join( - "/** @fileoverview", - " * @suppress {missingProvide|missingRequire}", - " */", - "goog.provide('module$mod$name');")), - SourceFile.fromCode( - Compiler.joinPathParts("base", "test", "sub.js"), - "goog.provide('module$test$sub'); goog.require('module$mod$name');"))); + SourceFile.fromCode(Compiler.joinPathParts("base", "mod", "name.js"), ""), + SourceFile.fromCode(Compiler.joinPathParts("base", "test", "sub.js"), ""))); } public void testUseImportInEs6ObjectLiteralShorthand() { testModules( "import {f} from './other.js';\nvar bar = {a: 1, f};", - LINE_JOINER.join( - "goog.require('module$other');", - "var bar$$module$testcode={a: 1, f: module$other.f};")); + "var bar$$module$testcode={a: 1, f: module$other.f};"); testModules( "import {f as foo} from './other.js';\nvar bar = {a: 1, foo};", - LINE_JOINER.join( - "goog.require('module$other');", - "var bar$$module$testcode={a: 1, foo: module$other.f};")); + "var bar$$module$testcode={a: 1, foo: module$other.f};"); testModules( "import f from './other.js';\nvar bar = {a: 1, f};", - LINE_JOINER.join( - "goog.require('module$other');", - "var bar$$module$testcode={a: 1, f: module$other.default};")); + "var bar$$module$testcode={a: 1, f: module$other.default};"); testModules( "import * as f from './other.js';\nvar bar = {a: 1, f};", - LINE_JOINER.join( - "goog.require('module$other');", - "var bar$$module$testcode={a: 1, f: module$other};")); + "var bar$$module$testcode={a: 1, f: module$other};"); } } diff --git a/test/com/google/javascript/jscomp/JSModuleGraphTest.java b/test/com/google/javascript/jscomp/JSModuleGraphTest.java index 82c36ed07a4..e4c592fe435 100644 --- a/test/com/google/javascript/jscomp/JSModuleGraphTest.java +++ b/test/com/google/javascript/jscomp/JSModuleGraphTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static java.util.Collections.shuffle; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -27,6 +28,7 @@ import java.util.Arrays; import java.util.BitSet; import java.util.List; +import java.util.HashMap; import junit.framework.TestCase; /** @@ -344,6 +346,105 @@ private List setUpManageDependenciesTest() { return inputs; } + public void testGoogBaseOrderedCorrectly() throws Exception { + List sourceFiles = new ArrayList<>(); + sourceFiles.add(code("a9", provides("a9"), requires())); + sourceFiles.add(code("a8", provides("a8"), requires())); + sourceFiles.add(code("a7", provides("a7"), requires())); + sourceFiles.add(code("a6", provides("a6"), requires())); + sourceFiles.add(code("a5", provides("a5"), requires())); + sourceFiles.add(code("a4", provides("a4"), requires())); + sourceFiles.add(code("a3", provides("a3"), requires())); + sourceFiles.add(code("a2", provides("a2"), requires())); + sourceFiles.add(code("a1", provides("a1"), requires("a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9"))); + sourceFiles.add(code("base.js", BASEJS, provides(), requires())); + + DependencyOptions depOptions = new DependencyOptions(); + depOptions.setDependencySorting(true); + depOptions.setDependencyPruning(true); + depOptions.setMoocherDropping(true); + depOptions.setEntryPoints(ImmutableList.of(ModuleIdentifier.forClosure("a1"))); + for (int i = 0; i < 10; i++) { + shuffle(sourceFiles); + A.removeAll(); + for (SourceFile sourceFile : sourceFiles) { + A.add(sourceFile); + } + + for (CompilerInput input : A.getInputs()) { + input.setCompiler(compiler); + } + + List inputs = new ArrayList<>(); + inputs.addAll(A.getInputs()); + List results = graph.manageDependencies(depOptions, inputs); + + assertInputs(A, "base.js", "a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9", "a1"); + + assertThat(sourceNames(results)) + .containsExactly("base.js", "a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9", "a1") + .inOrder(); + } + } + + public void testProperEs6ModuleOrdering() throws Exception { + List sourceFiles = new ArrayList<>(); + sourceFiles.add(code("/entry.js", provides(), requires())); + sourceFiles.add(code("/a/a.js", provides(), requires())); + sourceFiles.add(code("/a/b.js", provides(), requires())); + sourceFiles.add(code("/b/a.js", provides(), requires())); + sourceFiles.add(code("/b/b.js", provides(), requires())); + sourceFiles.add(code("/b/c.js", provides(), requires())); + sourceFiles.add(code("/important.js", provides(), requires())); + + HashMap> orderedRequires = new HashMap<>(); + orderedRequires.put("/entry.js", ImmutableList.of( + ModuleIdentifier.forFile("/b/b.js").toString(), + ModuleIdentifier.forFile("/b/a.js").toString(), + ModuleIdentifier.forFile("/important.js").toString(), + ModuleIdentifier.forFile("/a/b.js").toString(), + ModuleIdentifier.forFile("/a/a.js").toString())); + orderedRequires.put("/a/a.js", ImmutableList.of()); + orderedRequires.put("/a/b.js", ImmutableList.of()); + orderedRequires.put("/b/a.js", ImmutableList.of()); + orderedRequires.put("/b/b.js", ImmutableList.of( + ModuleIdentifier.forFile("/b/c.js").toString())); + orderedRequires.put("/b/c.js", ImmutableList.of()); + orderedRequires.put("/important.js", ImmutableList.of()); + + DependencyOptions depOptions = new DependencyOptions(); + depOptions.setDependencySorting(true); + depOptions.setDependencyPruning(true); + depOptions.setMoocherDropping(true); + depOptions.setEntryPoints(ImmutableList.of(ModuleIdentifier.forFile("/entry.js"))); + for (int iterationCount = 0; iterationCount < 10; iterationCount++) { + shuffle(sourceFiles); + A.removeAll(); + for (SourceFile sourceFile : sourceFiles) { + A.add(sourceFile); + } + + for (CompilerInput input : A.getInputs()) { + input.setCompiler(compiler); + for (String require : orderedRequires.get(input.getSourceFile().getName())) { + input.addOrderedRequire(require); + } + input.setHasFullParseDependencyInfo(true); + } + + List inputs = new ArrayList<>(); + inputs.addAll(A.getInputs()); + List results = graph.manageDependencies(depOptions, inputs); + + assertInputs(A, "/b/c.js", "/b/b.js", "/b/a.js", "/important.js", "/a/b.js", "/a/a.js", "/entry.js"); + + assertThat(sourceNames(results)) + .containsExactly("/b/c.js", "/b/b.js", "/b/a.js", "/important.js", "/a/b.js", "/a/a.js", "/entry.js") + .inOrder(); + } + } + + private void assertInputs(JSModule module, String... sourceNames) { assertEquals(ImmutableList.copyOf(sourceNames), sourceNames(module.getInputs())); } diff --git a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java index 90a5e415b6d..22ee39abea8 100644 --- a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java +++ b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java @@ -62,7 +62,6 @@ public void testWithoutExports() { "test.js", "var name = require('./other'); name()", LINE_JOINER.join( - "goog.require('module$other');", "var name = module$other;", "module$other();")); test( @@ -74,19 +73,11 @@ public void testWithoutExports() { "var name = require('../mod/name');", "(function() { module$mod$name(); })();"))), ImmutableList.of( - SourceFile.fromCode( - Compiler.joinPathParts("mod", "name.js"), - LINE_JOINER.join( - "/** @fileoverview", - " * @suppress {missingProvide|missingRequire}", - " */", - "goog.provide('module$mod$name');")), + SourceFile.fromCode(Compiler.joinPathParts("mod", "name.js"), ""), SourceFile.fromCode( Compiler.joinPathParts("test", "sub.js"), LINE_JOINER.join( - "goog.require('module$mod$name');", - "var name = module$mod$name;", - "(function() { module$mod$name(); })();")))); + "var name = module$mod$name;", "(function() { module$mod$name(); })();")))); } public void testExports() { @@ -96,8 +87,6 @@ public void testExports() { "var name = require('./other');", "exports.foo = 1;"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "/** @const */ var module$test = {};", "var name$$module$test = module$other;", "module$test.foo = 1;")); @@ -108,8 +97,6 @@ public void testExports() { "var name = require('./other');", "module.exports = function() {};"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "var name$$module$test = module$other;", "var module$test = function () {};")); } @@ -122,8 +109,6 @@ public void testExportsInExpression() { "var e;", "e = module.exports = function() {};"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "var module$test;", "var name$$module$test = module$other;", "var e$$module$test;", @@ -135,8 +120,6 @@ public void testExportsInExpression() { "var name = require('./other');", "var e = module.exports = function() {};"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "var module$test;", "var name$$module$test = module$other;", "var e$$module$test = module$test = function () {};")); @@ -147,8 +130,6 @@ public void testExportsInExpression() { "var name = require('./other');", "(module.exports = function() {})();"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "var module$test;", "var name$$module$test = module$other;", "(module$test = function () {})();")); @@ -162,7 +143,6 @@ public void testPropertyExports() { "module.exports.obj = {};", "module.exports.obj.two = 2;"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.one = 1;", "module$test.obj = {};", @@ -181,7 +161,6 @@ public void testModuleExportsWrittenWithExportsRefs() { "exports.one = 1;", "module.exports = {};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test={};", "module$test.one = 1;")); } @@ -192,7 +171,6 @@ public void testVarRenaming() { LINE_JOINER.join( "module.exports = {};", "var a = 1, b = 2;", "(function() { var a; b = 4})();"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test={};", "var a$$module$test = 1;", "var b$$module$test = 2;", @@ -206,8 +184,6 @@ public void testDash() { "var name = require('./other');", "exports.foo = 1;"), LINE_JOINER.join( - "goog.provide('module$test_test');", - "goog.require('module$other');", "/** @const */ var module$test_test = {};", "var name$$module$test_test = module$other;", "module$test_test.foo = 1;")); @@ -220,8 +196,6 @@ public void testIndex() { "var name = require('../other');", "exports.bar = 1;"), LINE_JOINER.join( - "goog.provide('module$foo$index');", - "goog.require('module$other');", "/** @const */ var module$foo$index={};", "var name$$module$foo$index = module$other;", "module$foo$index.bar = 1;")); @@ -238,7 +212,6 @@ public void testVarJsdocGoesOnAssignment() { "var MyEnum = { ONE: 1, TWO: 2 };", "module.exports = {MyEnum: MyEnum};"), LINE_JOINER.join( - "goog.provide('module$testcode');", "/** @const */", "var module$testcode = {};", "/**", @@ -255,8 +228,6 @@ public void testModuleName() { "var name = require('../other');", "module.exports = name;"), LINE_JOINER.join( - "goog.provide('module$foo$bar');", - "goog.require('module$other');", "var name$$module$foo$bar = module$other;", "var module$foo$bar = module$other;")); @@ -267,18 +238,10 @@ public void testModuleName() { Compiler.joinPathParts("foo", "bar.js"), LINE_JOINER.join("var name = require('./name');", "module.exports = name;"))), ImmutableList.of( - SourceFile.fromCode( - Compiler.joinPathParts("foo", "name.js"), - LINE_JOINER.join( - "/** @fileoverview", - " * @suppress {missingProvide|missingRequire}", - " */", - "goog.provide('module$foo$name');")), + SourceFile.fromCode(Compiler.joinPathParts("foo", "name.js"), ""), SourceFile.fromCode( Compiler.joinPathParts("foo", "bar.js"), LINE_JOINER.join( - "goog.provide('module$foo$bar');", - "goog.require('module$foo$name');", "var name$$module$foo$bar = module$foo$name;", "var module$foo$bar = module$foo$name;")))); } @@ -292,7 +255,6 @@ public void testModuleExportsScope() { "};", "module.exports = foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = function (module) {", " module.exports={};", "};")); @@ -306,7 +268,6 @@ public void testModuleExportsScope() { "};", "module.exports = foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = function() {", " var module={};", " module.exports={}", @@ -321,7 +282,6 @@ public void testModuleExportsScope() { "};", "module.exports = foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = function() {", " if (true) var module={};", " module.exports={}", @@ -340,9 +300,7 @@ public void testUMDPatternConversion() { "} else {", " this.foobar = foobar;", "}"), - LINE_JOINER.join( - "goog.provide('module$test');", - "var module$test = {foo: 'bar'};")); + "var module$test = {foo: 'bar'};"); testModules( "test.js", @@ -355,9 +313,7 @@ public void testUMDPatternConversion() { "} else {", " this.foobar = foobar;", "}"), - LINE_JOINER.join( - "goog.provide('module$test');", - "var module$test = {foo: 'bar'};")); + "var module$test = {foo: 'bar'};"); testModules( "test.js", @@ -369,9 +325,7 @@ public void testUMDPatternConversion() { "if (typeof define === 'function' && define.amd) {", " define([], function () {return foobar;});", "}"), - LINE_JOINER.join( - "goog.provide('module$test');", - "var module$test = {foo: 'bar'};")); + "var module$test = {foo: 'bar'};"); } public void testEs6ObjectShorthand() { @@ -386,7 +340,6 @@ public void testEs6ObjectShorthand() { " foo", "};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.foo = function () {};", "module$test.prop = 'value';")); @@ -401,7 +354,6 @@ public void testEs6ObjectShorthand() { " }", "};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.prop = 'value';", "module$test.foo = function() {", @@ -414,8 +366,6 @@ public void testEs6ObjectShorthand() { "var a = require('./other');", "module.exports = {a: a};"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "/** @const */ var module$test = {};", "var a$$module$test = module$other;", "module$test.a = module$other;")); @@ -426,8 +376,6 @@ public void testEs6ObjectShorthand() { "var a = require('./other');", "module.exports = {a};"), LINE_JOINER.join( - "goog.provide('module$test');", - "goog.require('module$other');", "/** @const */ var module$test = {};", "var a$$module$test = module$other;", "module$test.a = module$other;")); @@ -438,7 +386,6 @@ public void testEs6ObjectShorthand() { "var a = 4;", "module.exports = {a};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.a = 4;")); } @@ -450,13 +397,12 @@ public void testKeywordsInExports() { "var a = 4;", "module.exports = { else: a };"), LINE_JOINER.join( - "goog.provide('module$testcode');", "/** @const */ var module$testcode = {};", "module$testcode.else = 4;")); } public void testRequireResultUnused() { - testModules("test.js", "require('./other');", "goog.require('module$other');"); + testModules("test.js", "require('./other');", ""); } public void testRequireEnsure() { @@ -468,7 +414,6 @@ public void testRequireEnsure() { " var bar = other;", "});"), LINE_JOINER.join( - "goog.require('module$other');", "(function() {", " var other=module$other;", " var bar = module$other;", @@ -483,7 +428,6 @@ public void testFunctionRewriting() { "foo.prototype = new Date();", "module.exports = foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = function() {};", "module$test.prototype = new Date();")); @@ -494,7 +438,6 @@ public void testFunctionRewriting() { "foo.prototype = new Date();", "module.exports = {foo: foo};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.foo = function () {}", "module$test.foo.prototype = new Date();")); @@ -508,7 +451,6 @@ public void testFunctionHoisting() { "function foo() {}", "foo.prototype = new Date();"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = function() {};", "module$test.prototype = new Date();")); @@ -521,7 +463,6 @@ public void testFunctionHoisting() { "module.exports = foo;", "module.exports.bar = foobar;"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = function () {};", "module$test.bar = function() {};", "Object.assign(module$test, { bar: module$test.bar });")); @@ -532,21 +473,13 @@ public void testClassRewriting() { CompilerOptions.LanguageMode.ECMASCRIPT_2015, CompilerOptions.LanguageMode.ECMASCRIPT5); testModules( "test.js", - LINE_JOINER.join( - "class foo extends Array {}", - "module.exports = foo;"), - LINE_JOINER.join( - "goog.provide('module$test');", - "let module$test = class extends Array {}")); + LINE_JOINER.join("class foo extends Array {}", "module.exports = foo;"), + "let module$test = class extends Array {}"); testModules( "test.js", - LINE_JOINER.join( - "class foo {}", - "module.exports = foo;"), - LINE_JOINER.join( - "goog.provide('module$test');", - "let module$test = class {}")); + LINE_JOINER.join("class foo {}", "module.exports = foo;"), + "let module$test = class {}"); testModules( "test.js", @@ -554,7 +487,6 @@ public void testClassRewriting() { "class foo {}", "module.exports.foo = foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.foo = class {};")); @@ -566,7 +498,6 @@ public void testClassRewriting() { " bar() { return 'bar'; }", "};"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = class {", " /** @this {module$test} */", " bar() { return 'bar'; }", @@ -586,7 +517,6 @@ public void testMultipleAssignments() { "Bar.prototype.foobar = function() { alert('foobar'); };", "exports = Bar;"), LINE_JOINER.join( - "goog.provide('module$test')", "var module$test = /** @constructor */ function(){};", "/** @constructor */ function Bar$$module$test(){}", "Bar$$module$test.prototype.foobar = function() { alert('foobar'); };", @@ -602,7 +532,6 @@ public void testDestructuringImports() { "const {foo, bar} = require('./other');", "var baz = foo + bar;"), LINE_JOINER.join( - "goog.require('module$other');", "const {foo, bar} = module$other;", "var baz = module$other.foo + module$other.bar;")); } @@ -617,7 +546,6 @@ public void testAnnotationsCopied() { "/** @type {string} */ a.prototype.foo;", "module.exports.a = a;"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "/** @interface */ module$test.a;", "/** @type {string} */ module$test.a.prototype.foo;")); @@ -636,9 +564,7 @@ public void testUMDRemoveIIFE() { "} else {", " this.foobar = foobar;", "}})()"), - LINE_JOINER.join( - "goog.provide('module$test');", - "var module$test = {foo: 'bar'};")); + "var module$test = {foo: 'bar'};"); testModules( "test.js", @@ -652,9 +578,7 @@ public void testUMDRemoveIIFE() { "} else {", " this.foobar = foobar;", "}})()"), - LINE_JOINER.join( - "goog.provide('module$test');", - "var module$test = {foo: 'bar'};")); + "var module$test = {foo: 'bar'};"); testModules( "test.js", @@ -668,9 +592,7 @@ public void testUMDRemoveIIFE() { "} else {", " this.foobar = foobar;", "}}.call(this))"), - LINE_JOINER.join( - "goog.provide('module$test');", - "var module$test = {foo: 'bar'};")); + "var module$test = {foo: 'bar'};"); testModules( "test.js", @@ -686,7 +608,6 @@ public void testUMDRemoveIIFE() { " global.foobar = foobar;", "}})(this)"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = {foo: 'bar'};", "this.foobar = module$test;")); @@ -704,7 +625,6 @@ public void testUMDRemoveIIFE() { " global.foobar = foobar;", "}}.call(this, this))"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = {foo: 'bar'};", "this.foobar = module$test;")); @@ -722,7 +642,6 @@ public void testUMDRemoveIIFE() { " this.foobar = foobar;", "}}.call(window))"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = {};", "(function(){", " module$test={foo:\"bar\"};", @@ -743,7 +662,6 @@ public void testUMDRemoveIIFE() { "}})();", "alert('foo');"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = {};", "(function(){", " module$test={foo:\"bar\"};", @@ -765,7 +683,6 @@ public void testUMDRemoveIIFE() { " this.foobar = foobar;", "}})();"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = {};", "alert('foo');", "(function(){", @@ -789,7 +706,6 @@ public void testUMDRemoveIIFE() { " global.foobar = foobar;", "}}.call(this, this))"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @param {...*} var_args */", "function log$$module$test(var_args){}", "var module$test = {", @@ -808,7 +724,6 @@ public void testParamShadow() { "Foo.prototype.test = new Bar(Foo);", "module.exports = Foo;"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = /** @constructor */ function () {};", "/** @constructor */ function Bar$$module$test(Foo) { this.foo = new Foo(); }", "module$test.prototype.test = new Bar$$module$test(module$test);")); @@ -819,7 +734,6 @@ public void testIssue2308() { "test.js", "exports.y = null; var x; x = exports.y;", LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {};", "module$test.y = null;", "var x$$module$test;", @@ -834,22 +748,13 @@ public void testAbsoluteImportsWithModuleRoots() { SourceFile.fromCode( Compiler.joinPathParts("base", "test", "sub.js"), LINE_JOINER.join( - "var name = require('/mod/name');", - "(function() { module$mod$name(); })();"))), + "var name = require('/mod/name');", "(function() { module$mod$name(); })();"))), ImmutableList.of( - SourceFile.fromCode( - Compiler.joinPathParts("base", "mod", "name.js"), - LINE_JOINER.join( - "/** @fileoverview", - " * @suppress {missingProvide|missingRequire}", - " */", - "goog.provide('module$mod$name');")), + SourceFile.fromCode(Compiler.joinPathParts("base", "mod", "name.js"), ""), SourceFile.fromCode( Compiler.joinPathParts("base", "test", "sub.js"), LINE_JOINER.join( - "goog.require('module$mod$name');", - "var name = module$mod$name;", - "(function() { module$mod$name(); })();")))); + "var name = module$mod$name;", "(function() { module$mod$name(); })();")))); } public void testIssue2510() { @@ -861,7 +766,6 @@ public void testIssue2510() { " get b() { return 2; }", "};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test = {", " get b() { return 2; }", "}", @@ -880,7 +784,6 @@ public void testIssue2450() { " HASHSIZE: BCRYPT_HASHSIZE,", "};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test={};", "module$test.BLOCKS = 8;", "module$test.HASHSIZE = 32;")); @@ -898,7 +801,6 @@ public void testWebpackAmdPattern() { " __WEBPACK_AMD_DEFINE_RESULT__ !== undefined", " && (module.exports = __WEBPACK_AMD_DEFINE_RESULT__));"), LINE_JOINER.join( - "goog.provide('module$test');", "var module$test = {};", "var __WEBPACK_AMD_DEFINE_ARRAY__$$module$test;", "!(__WEBPACK_AMD_DEFINE_ARRAY__$$module$test = ", @@ -920,7 +822,6 @@ public void testIssue2593() { "", "module.exports = {};"), LINE_JOINER.join( - "goog.provide('module$test');", "/** @const */ var module$test={};", "var first$$module$test=1;", "var second$$module$test=2;", From 47fbd16a46dccc05d181699649a4c6eb2a41d636 Mon Sep 17 00:00:00 2001 From: Chad Killingsworth Date: Wed, 4 Oct 2017 12:11:37 -0500 Subject: [PATCH 2/2] Build a map of provides to inputs so that lookups are constant time. --- .../javascript/jscomp/JSModuleGraph.java | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/com/google/javascript/jscomp/JSModuleGraph.java b/src/com/google/javascript/jscomp/JSModuleGraph.java index ed97a4a409b..0677d0fb2dd 100644 --- a/src/com/google/javascript/jscomp/JSModuleGraph.java +++ b/src/com/google/javascript/jscomp/JSModuleGraph.java @@ -445,6 +445,17 @@ public List manageDependencies( Iterable entryPointInputs = createEntryPointInputs( depOptions, inputs, sorter); + HashMap inputsByProvide = new HashMap<>(); + for (CompilerInput input : inputs) { + for (String provide : input.getKnownProvides()) { + inputsByProvide.put(provide, input); + } + String moduleName = input.getPath().toModuleName(); + if (!inputsByProvide.containsKey(moduleName)) { + inputsByProvide.put(moduleName, input); + } + } + // The order of inputs, sorted independently of modules. List absoluteOrder = sorter.getDependenciesOf(inputs, depOptions.shouldSortDependencies()); @@ -478,7 +489,8 @@ public List manageDependencies( // We need the ful set of dependencies for each module, so start with the full input set Set inputsNotYetReached = new HashSet<>(inputs); for (CompilerInput entryPoint : entryPointInputsPerModule.get(module)) { - transitiveClosure.addAll(getDepthFirstDependenciesOf(entryPoint, inputsNotYetReached)); + transitiveClosure.addAll( + getDepthFirstDependenciesOf(entryPoint, inputsNotYetReached, inputsByProvide)); } // For any input we have not yet reached, add them to the ordered list for (CompilerInput orderedInput : transitiveClosure) { @@ -532,17 +544,23 @@ public List manageDependencies( * performing a recursive, depth-first traversal. */ private List getDepthFirstDependenciesOf( - CompilerInput rootInput, Set unreachedInputs) { + CompilerInput rootInput, + Set unreachedInputs, + Map inputsByProvide) { List orderedInputs = new ArrayList<>(); if (!unreachedInputs.remove(rootInput)) { return orderedInputs; } for (String importedNamespace : rootInput.getRequires()) { - CompilerInput dependency = - JSModuleGraph.findInputProviding(importedNamespace, unreachedInputs); + CompilerInput dependency = null; + if (inputsByProvide.containsKey(importedNamespace) + && unreachedInputs.contains(inputsByProvide.get(importedNamespace))) { + dependency = inputsByProvide.get(importedNamespace); + } + if (dependency != null) { - orderedInputs.addAll(getDepthFirstDependenciesOf(dependency, unreachedInputs)); + orderedInputs.addAll(getDepthFirstDependenciesOf(dependency, unreachedInputs, inputsByProvide)); } } @@ -550,19 +568,6 @@ private List getDepthFirstDependenciesOf( return orderedInputs; } - private static CompilerInput findInputProviding(String namespace, Set inputs) { - for (CompilerInput input : inputs) { - if (namespace.startsWith("module$")) { - if (input.getPath().toModuleName().equals(namespace)) { - return input; - } - } else if (input.getProvides().contains(namespace)) { - return input; - } - } - return null; - } - private Collection createEntryPointInputs( DependencyOptions depOptions, List inputs,