Skip to content

Commit

Permalink
If a commonjs module import cannot be located, provide a fallback
Browse files Browse the repository at this point in the history
Provide a fallback mechanism so we don't leave a stray `require()` call.
Useful for whitespace_only mode and per-file rewriting.

Closes #2695

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=176438020
  • Loading branch information
ChadKillingsworth authored and lauraharker committed Nov 21, 2017
1 parent 8a26437 commit c98a23b
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 24 deletions.
49 changes: 26 additions & 23 deletions src/com/google/javascript/jscomp/ProcessCommonJSModules.java
Expand Up @@ -401,27 +401,22 @@ public void visit(NodeTraversal t, Node n, Node parent) {

/** Visit require calls. */
private void visitRequireCall(NodeTraversal t, Node require, Node parent) {
String requireName = ProcessCommonJSModules.getCommonJsImportPath(require);
ModulePath modulePath =
t.getInput()
.getPath()
.resolveJsModule(
requireName,
require.getSourceFileName(),
require.getLineno(),
require.getCharno());
if (modulePath == null) {
// The module loader will issue an error
return;
}

// 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 dependency
// sorting will insert the file for us.
if (!NodeUtil.isExpressionResultUsed(require)
&& parent.isExprResult()
&& NodeUtil.isStatementBlock(parent.getParent())) {

// Attempt to resolve the module so that load warnings are issued
t.getInput()
.getPath()
.resolveJsModule(
getCommonJsImportPath(require),
require.getSourceFileName(),
require.getLineno(),
require.getCharno());
Node grandparent = parent.getParent();
parent.detach();
compiler.reportChangeToEnclosingScope(grandparent);
Expand Down Expand Up @@ -912,12 +907,14 @@ private void visitRequireCall(NodeTraversal t, Node require, Node parent) {
require.getSourceFileName(),
require.getLineno(),
require.getCharno());

String moduleName;
if (modulePath == null) {
// The module loader will issue an error
return;
// The module loader will issue an error, but use a fallback
moduleName = ModuleIdentifier.forFile(requireName).getModuleName();
} else {
moduleName = modulePath.toModuleName();
}

String moduleName = modulePath.toModuleName();
Node moduleRef = IR.name(moduleName).srcref(require);
parent.replaceChild(require, moduleRef);

Expand Down Expand Up @@ -1436,10 +1433,13 @@ private String getModuleImportName(NodeTraversal t, Node n) {
.getPath()
.resolveJsModule(
requireName, n.getSourceFileName(), n.getLineno(), n.getCharno());
String moduleName;
if (modulePath == null) {
return null;
moduleName = ModuleIdentifier.forFile(requireName).getModuleName();
} else {
moduleName = modulePath.toModuleName();
}
return modulePath.toModuleName() + propSuffix;
return moduleName + propSuffix;
}
return null;

Expand Down Expand Up @@ -1480,12 +1480,15 @@ private void fixTypeNode(NodeTraversal t, Node typeNode) {
typeNode.getSourceFileName(),
typeNode.getLineno(),
typeNode.getCharno());

String globalModuleName;
if (modulePath == null) {
// The module loader will issue an error
return;
// The module loader will issue an error, but we fall back to a path-based name
globalModuleName = ModuleIdentifier.forFile(moduleName).getModuleName();
} else {
globalModuleName = modulePath.toModuleName();
}

String globalModuleName = modulePath.toModuleName();
typeNode.setString(
localTypeName == null ? globalModuleName : globalModuleName + localTypeName);

Expand Down
Expand Up @@ -178,6 +178,7 @@ jsdoc.suppressions =\
missingProvide,\
missingRequire,\
missingReturn,\
moduleLoad,\
newCheckTypes,\
newCheckTypesAllChecks,\
nonStandardJsDocs,\
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/resources.json

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java
Expand Up @@ -941,5 +941,20 @@ public void testIssue2616() {

public void testMissingRequire() {
ModulesTestUtils.testModulesError(this, "require('missing');", ModuleLoader.LOAD_WARNING);

testModules(
"test.js",
lines(
"/**",
" * @fileoverview",
" * @suppress {moduleLoad}",
" */",
"var foo = require('missing');"),
lines(
"/**",
" * @fileoverview",
" * @suppress {moduleLoad}",
" */",
"var foo = module$missing;"));
}
}

0 comments on commit c98a23b

Please sign in to comment.