Skip to content

Commit

Permalink
Improve CommonJS rewriting when used with webpack
Browse files Browse the repository at this point in the history 
Recognize module['exports'] references as exports.
Properly rewrite ES Modules transpiled by older Babel versions where both `module.exports` and `exports` are referenced in the same file.
Better recognize AMD shims added by webpack. Webpack does not update require statements in dead trees of a UMD if block. Don't report module load errors until after then UMD patterns have been replaced.

Closes #2989

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202327240
  • Loading branch information
@ChadKillingsworth @brad4d
ChadKillingsworth authored and brad4d committed Jun 28, 2018
1 parent 15d7004 commit 3edafa4
Show file tree
Hide file tree
Showing 2 changed files with 269 additions and 78 deletions.
199 changes: 136 additions & 63 deletions src/com/google/javascript/jscomp/ProcessCommonJSModules.java
View file
Expand Up @@ -30,6 +30,7 @@
import com.google.javascript.rhino.JSDocInfoBuilder;
import com.google.javascript.rhino.Node;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -84,6 +85,8 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
}

FindImportsAndExports finder = new FindImportsAndExports();
ErrorHandler moduleLoaderErrorHandler = compiler.getModuleLoader().getErrorHandler();
compiler.getModuleLoader().setErrorHandler(finder);
NodeTraversal.traverse(compiler, n, finder);

CompilerInput.ModuleType moduleType = compiler.getInput(n.getInputId()).getJsModuleType();
Expand All @@ -93,11 +96,9 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {

boolean isCommonJsModule = finder.isCommonJsModule();
ImmutableList.Builder<ExportInfo> exports = ImmutableList.builder();
boolean needsRetraverse = false;
if (isCommonJsModule || forceModuleDetection) {
finder.reportModuleErrors();

if (!finder.umdPatterns.isEmpty()) {
boolean needsRetraverse = false;
if (finder.replaceUmdPatterns()) {
needsRetraverse = true;
}
Expand All @@ -109,27 +110,18 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {

if (needsRetraverse) {
finder = new FindImportsAndExports();
compiler.getModuleLoader().setErrorHandler(finder);
NodeTraversal.traverse(compiler, n, finder);
}
}

defaultExportIsConst = finder.initializeModule();

//UMD pattern replacement can leave detached export references - don't include those
for (ExportInfo export : finder.getModuleExports()) {
if (NodeUtil.getEnclosingScript(export.node) != null) {
exports.add(export);
}
}
for (ExportInfo export : finder.getExports()) {
if (NodeUtil.getEnclosingScript(export.node) != null) {
exports.add(export);
}
}
} else if (needsRetraverse) {
finder = new FindImportsAndExports();
NodeTraversal.traverse(compiler, n, finder);
exports.addAll(finder.getModuleExports());
exports.addAll(finder.getExports());
}
finder.reportModuleErrors();
compiler.getModuleLoader().setErrorHandler(moduleLoaderErrorHandler);

NodeTraversal.traverse(
compiler,
Expand Down Expand Up @@ -247,7 +239,11 @@ private String getImportedModuleName(NodeTraversal t, Node n, String importPath)
*/
public static boolean isCommonJsExport(
NodeTraversal t, Node export, ModuleLoader.ResolutionMode resolutionMode) {
if (export.matchesQualifiedName(MODULE + "." + EXPORTS)) {
if (export.matchesQualifiedName(MODULE + "." + EXPORTS)
|| (export.isGetElem()
&& export.getFirstChild().matchesQualifiedName(MODULE)
&& export.getSecondChild().isString()
&& export.getSecondChild().getString().equals(EXPORTS))) {
Var v = t.getScope().getVar(MODULE);
if (v == null || v.isExtern()) {
return true;
Expand Down Expand Up @@ -523,7 +519,7 @@ && isCommonJsImport(arg.getFirstChild())
* Traverse the script. Find all references to CommonJS require (import) and module.exports or
* export statements. Rewrites any require calls to reference the rewritten module name.
*/
class FindImportsAndExports implements NodeTraversal.Callback {
class FindImportsAndExports implements NodeTraversal.Callback, ErrorHandler {
private boolean hasGoogProvideOrModule = false;
private Node script = null;

Expand All @@ -536,6 +532,11 @@ boolean isCommonJsModule() {
List<ExportInfo> exports = new ArrayList<>();
List<JSError> errors = new ArrayList<>();

@Override
public void report(CheckLevel ignoredLevel, JSError error) {
errors.add(error);
}

public List<ExportInfo> getModuleExports() {
return ImmutableList.copyOf(moduleExports);
}
Expand Down Expand Up @@ -576,42 +577,37 @@ public void visit(NodeTraversal t, Node n, Node parent) {
visitRequireEnsureCall(t, n);
}

if (n.matchesQualifiedName(MODULE + "." + EXPORTS)) {
if (n.matchesQualifiedName(MODULE + "." + EXPORTS)
|| (n.isGetElem()
&& n.getFirstChild().matchesQualifiedName(MODULE)
&& n.getSecondChild().isString()
&& n.getSecondChild().getString().equals(EXPORTS))) {
if (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,
// If the module.exports statement is nested in an if statement,
// and the test of the if checks for "module" or "define,
// assume the if statement is an UMD pattern with a common js export in the then branch
Node ifAncestor = getOutermostUmdTest(parent);
if (ifAncestor != null && (NodeUtil.isLValue(n) || isInIfTest(n))) {
UmdPattern existingPattern = findUmdPattern(umdPatterns, ifAncestor);
if (existingPattern != null) {
umdPatterns.remove(existingPattern);
UmdTestInfo umdTestAncestor = getOutermostUmdTest(parent);
if (umdTestAncestor != null && !isInIfTest(n)) {
UmdPattern existingPattern = findUmdPattern(umdPatterns, umdTestAncestor.enclosingIf);
if (existingPattern == null) {
umdPatterns.add(
new UmdPattern(umdTestAncestor.enclosingIf, umdTestAncestor.activeBranch));
}
Node enclosingIf =
NodeUtil.getEnclosingNode(
n,
new Predicate<Node>() {
@Override
public boolean apply(Node node) {
return node.isIf() || node.isHook();
}
});
umdPatterns.add(new UmdPattern(ifAncestor, enclosingIf.getSecondChild()));
}
}
} else if (n.matchesQualifiedName("define.amd")
|| n.matchesQualifiedName("window.define.amd")) {
// If a define.amd statement is nested in the then branch of an if statement,
// and the test of the if checks for "module" or "define,
// assume the if statement is an UMD pattern with a common js export
// in the else branch
Node ifAncestor = getOutermostUmdTest(parent);
if (ifAncestor != null
&& findUmdPattern(umdPatterns, ifAncestor) == null
&& (NodeUtil.isLValue(n) || isInIfTest(n))) {
umdPatterns.add(new UmdPattern(ifAncestor, ifAncestor.getChildAtIndex(2)));
// If a define.amd statement is in the test of an if statement,
// and the statement has no "else" block, we simply want to remove
// the entire if statement.
UmdTestInfo umdTestAncestor = getOutermostUmdTest(parent);
if (umdTestAncestor != null && isInIfTest(n)) {
UmdPattern existingPattern = findUmdPattern(umdPatterns, umdTestAncestor.enclosingIf);
if (existingPattern == null && umdTestAncestor.enclosingIf.getChildAtIndex(2) == null) {
umdPatterns.add(new UmdPattern(umdTestAncestor.enclosingIf, null));
}
}
}

Expand All @@ -633,7 +629,12 @@ && findUmdPattern(umdPatterns, ifAncestor) == null
.getFirstChild()
.matchesQualifiedName(MODULE + "." + EXPORTS)))) {
exports.add(new ExportInfo(n, t.getScope()));
} else if (!this.hasGoogProvideOrModule) {

// Ignore inlining created identity vars
// var exports = exports
} else if (!this.hasGoogProvideOrModule
&& (v == null
|| (v.getNameNode() == null && v.getNameNode().getFirstChild() != n))) {
errors.add(t.makeError(qNameRoot, SUSPICIOUS_EXPORTS_ASSIGNMENT));
}
} else {
Expand All @@ -642,11 +643,13 @@ && findUmdPattern(umdPatterns, ifAncestor) == null
// If the exports statement is nested in the then branch of an if statement,
// and the test of the if checks for "module" or "define,
// assume the if statement is an UMD pattern with a common js export in the then branch
Node ifAncestor = getOutermostUmdTest(parent);
if (ifAncestor != null
&& findUmdPattern(umdPatterns, ifAncestor) == null
&& (NodeUtil.isLValue(n) || isInIfTest(n))) {
umdPatterns.add(new UmdPattern(ifAncestor, ifAncestor.getSecondChild()));
UmdTestInfo umdTestAncestor = getOutermostUmdTest(parent);
if (umdTestAncestor != null && !isInIfTest(n)) {
UmdPattern existingPattern = findUmdPattern(umdPatterns, umdTestAncestor.enclosingIf);
if (existingPattern == null) {
umdPatterns.add(
new UmdPattern(umdTestAncestor.enclosingIf, umdTestAncestor.activeBranch));
}
}
}
}
Expand Down Expand Up @@ -772,6 +775,7 @@ boolean initializeModule() {
List<ExportInfo> exportsToRemove = new ArrayList<>();
for (ExportInfo export : exports) {
if (NodeUtil.getEnclosingScript(export.node) == null) {
exportsToRemove.add(export);
continue;
}
Node qNameBase = getBaseQualifiedNameNode(export.node);
Expand Down Expand Up @@ -810,10 +814,45 @@ boolean initializeModule() {
exportsToRemove.add(export);
compiler.reportChangeToEnclosingScope(assign);
}
// Find babel transpiled interop assignment
// module.exports = exports['default'];
} else if (export.node.getParent().isGetElem()
&& export.node.getNext().isString()
&& export.node.getNext().getString().equals(EXPORT_PROPERTY_NAME)
&& export.node.getGrandparent().isAssign()
&& export.node.getParent().getPrevious() != null
&& export.node.getParent().getPrevious().matchesQualifiedName(MODULE + "." + EXPORTS)) {
Node parent = export.node.getParent();
Node grandparent = parent.getParent();
Node prop = export.node.getNext();
parent.replaceWith(
IR.getprop(export.node.detach(), prop.detach()).useSourceInfoFrom(parent));

compiler.reportChangeToEnclosingScope(grandparent);
}
}

exports.removeAll(exportsToRemove);
exportsToRemove.clear();
HashMap<ExportInfo, ExportInfo> exportsToReplace = new HashMap<>();
for (ExportInfo export : moduleExports) {
if (NodeUtil.getEnclosingScript(export.node) == null) {
exportsToRemove.add(export);
} else if (export.node.isGetElem()) {
Node prop = export.node.getSecondChild().detach();
ExportInfo newExport =
new ExportInfo(IR.getprop(export.node.getFirstChild().detach(), prop), export.scope);
export.node.replaceWith(newExport.node);
compiler.reportChangeToEnclosingScope(newExport.node);
exportsToReplace.put(export, newExport);
}
}
moduleExports.removeAll(exportsToRemove);
for (ExportInfo oldExport : exportsToReplace.keySet()) {
int oldIndex = moduleExports.indexOf(oldExport);
moduleExports.remove(oldIndex);
moduleExports.add(oldIndex, exportsToReplace.get(oldExport));
}

// If we assign to the variable more than once or all the assignments
// are properties, initialize the variable as well.
Expand Down Expand Up @@ -844,26 +883,38 @@ boolean initializeModule() {
JSDocInfoBuilder builder = new JSDocInfoBuilder(true);
builder.recordConstancy();
initModule.setJSDocInfo(builder.build());
if (directAssignments == 0) {
if (directAssignments == 0 || (!exports.isEmpty() && !moduleExports.isEmpty())) {
Node defaultProp = IR.stringKey(EXPORT_PROPERTY_NAME);
defaultProp.putBooleanProp(Node.MODULE_EXPORT, true);
defaultProp.addChildToFront(IR.objectlit());
initModule.getFirstFirstChild().addChildToFront(defaultProp);
builder = new JSDocInfoBuilder(true);
builder.recordConstancy();
defaultProp.setJSDocInfo(builder.build());
if (exports.isEmpty() || moduleExports.isEmpty()) {
builder = new JSDocInfoBuilder(true);
builder.recordConstancy();
defaultProp.setJSDocInfo(builder.build());
}
}
this.script.addChildToFront(initModule.useSourceInfoFromForTree(this.script));
compiler.reportChangeToEnclosingScope(this.script);

return directAssignments < 2;
return directAssignments < 2 && (exports.isEmpty() || moduleExports.isEmpty());
}

private class UmdTestInfo {
public Node enclosingIf;
public Node activeBranch;

UmdTestInfo(Node enclosingIf, Node activeBranch) {
this.enclosingIf = enclosingIf;
this.activeBranch = activeBranch;
}
}

/**
* Find the outermost if node ancestor for a node without leaving the function scope. To match,
* the test class of the "if" statement must reference "module" or "define" names.
*/
private Node getOutermostUmdTest(Node n) {
private UmdTestInfo getOutermostUmdTest(Node n) {
if (n == null || NodeUtil.isTopLevel(n) || n.isFunction()) {
return null;
}
Expand All @@ -874,10 +925,21 @@ private Node getOutermostUmdTest(Node n) {

// When walking up ternary operations (hook), don't check if parent is the condition,
// because one ternary operation can be then/else branch of another.
if (parent.isIf() || parent.isHook()) {
Node outerIf = getOutermostUmdTest(parent);
if (outerIf != null) {
return outerIf;
if ((parent.isIf() || parent.isHook())) {
UmdTestInfo umdTestInfo = getOutermostUmdTest(parent);
if (umdTestInfo != null) {
// If this is the then block of an else-if statement, set the active branch to the
// then block rather than the "if" itself.
if (parent.isIf()
&& parent.getSecondChild() == n
&& parent.getParent() != null
&& parent.getParent().isBlock()
&& parent.getNext() == null
&& umdTestInfo.activeBranch == parent.getParent()) {
return new UmdTestInfo(umdTestInfo.enclosingIf, n);
}

return umdTestInfo;
}

final List<Node> umdTests = new ArrayList<>();
Expand All @@ -888,7 +950,10 @@ private Node getOutermostUmdTest(Node n) {
public void visit(Node node) {
if ((node.isName()
&& (node.getString().equals(MODULE) || node.getString().equals("define")))
|| (node.isGetProp() && node.matchesQualifiedName("window.define"))) {
|| (node.isGetProp() && node.matchesQualifiedName("window.define"))
|| (node.isString()
&& node.getString().equals("amd")
&& node.getParent().isIn())) {
umdTests.add(node);
}
}
Expand All @@ -900,7 +965,7 @@ public void visit(Node node) {
}

if (!umdTests.isEmpty()) {
return parent;
return new UmdTestInfo(parent, n);
}

return null;
Expand Down Expand Up @@ -1271,6 +1336,14 @@ public boolean apply(Node node) {
maybeUpdateName(t, n, nameDeclaration);
}
}
// Replace loose "module" references with an object literal
} else if (n.getString().equals(MODULE)
&& !(n.getParent().isGetProp()
|| n.getParent().isGetElem()
|| n.getParent().isTypeOf())) {
Node objectLit = IR.objectlit().useSourceInfoFrom(n);
n.replaceWith(objectLit);
t.reportCodeChange(objectLit);
}
break;
}
Expand Down

0 comments on commit 3edafa4

Please sign in to comment.