Partial rollback of "Make ES6 module bundling work with different mod…

…ule resolution schemes."

The issue with the change is that registered paths are no longer escaped,
but import paths are.  So these paths don't line up.
In order to roll this forward it requires a semi large reworking of the module
loader to be configured to not escape paths.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197468860

src/com/google/javascript/jscomp/Es6RewriteModulesToCommonJsModules.java

@@ -20,7 +20,6 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
-import com.google.javascript.jscomp.deps.ModuleLoader;
 import com.google.javascript.jscomp.deps.ModuleLoader.ModulePath;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
@@ -83,21 +82,6 @@ private static class LocalQName {
     }
   }
 
-  /**
-   * Normalizes a registered or import path.
-   *
-   * <p>Absolute import paths need to match the registered path exactly. Some {@link
-   * ModuleLoader.ModulePath#resolveModuleAsPath(String)} will have a leading slash and some won't.
-   * Ditto with registered paths (depends on your --js arguments). So in order to have everything
-   * line up AND preserve schemes (if they exist) then just strip leading /.
-   */
-  private static String normalizePath(String path) {
-    if (path.startsWith("/")) {
-      return path.substring(1);
-    }
-    return path;
-  }
-
   /**
    * Rewrites a single ES6 module into a CommonJS like module designed to be loaded in the
    * compiler's module runtime.
@@ -126,7 +110,7 @@ private class Rewriter extends AbstractPostOrderCallback {
     public void visit(NodeTraversal t, Node n, Node parent) {
       switch (n.getToken()) {
         case IMPORT:
-          visitImport(t.getInput().getPath(), n);
+          visitImport(n);
           break;
         case EXPORT:
           visitExport(t, n, parent);
@@ -309,16 +293,11 @@ private void registerAndLoadModule(NodeTraversal t) {
               IR.call(
                   IR.getprop(IR.name("$jscomp"), IR.string("registerAndLoadModule")),
                   moduleFunction,
-                  // By default the input's path is escaped, so make a new path that isn't escaped
-                  // in order to preserve characters that may be invalid on certain file systems but
-                  // valid in a browser URL. Resolving this path also enables removing module roots
-                  // from this path.
-                  IR.string(
-                      normalizePath(
-                          compiler
-                              .getModuleLoader()
-                              .resolveWithoutEscapingPath(t.getInput().getName())
-                              .toString())),
+                  // Specifically use the input's name rather than modulePath.toString(). The former
+                  // is the raw path and the latter is encoded (special characters are replaced).
+                  // This is designed to run in a web browser and we want to preserve the URL given
+                  // to us. But the encodings will replace : with - due to windows.
+                  IR.string(t.getInput().getName()),
                   shallowDeps));
 
       script.addChildToBack(exprResult.useSourceInfoIfMissingFromForTree(script));
@@ -362,12 +341,8 @@ private void addExport(Node definePropertiesLit, String exportedName, LocalQName
       compiler.reportChangeToChangeScope(getterFunction);
     }
 
-    private void visitImport(ModuleLoader.ModulePath path, Node importDecl) {
-      // Normalize the import path according to the module resolution scheme so that bundles are
-      // compatible with the compiler's module loader options.
-      importRequests.add(
-          normalizePath(
-              path.resolveModuleAsPath(importDecl.getLastChild().getString()).toString()));
+    private void visitImport(Node importDecl) {
+      importRequests.add(importDecl.getLastChild().getString());
       imports.add(importDecl);
     }
 

test/com/google/javascript/jscomp/Es6RewriteModulesToCommonJsModulesTest.java

@@ -475,7 +475,8 @@ public void testFileNameIsPreserved() {
                     "}, 'https://example.domain.google.com/test.js', []);"))));
   }
 
-  public void testRegisteredPathDoesNotIncludeModuleRoot() {
+  // TODO(johnplaisted): This should strip module roots
+  public void testRegisteredPathDoesIncludeModuleRoot() {
     moduleRoots = ImmutableList.of("module/root/");
 
     test(
@@ -486,10 +487,11 @@ public void testRegisteredPathDoesNotIncludeModuleRoot() {
                 lines(
                     "$jscomp.registerAndLoadModule(function($$require, $$exports, $$module) {",
                     "  'test pragma';",
-                    "}, 'test.js', []);"))));
+                    "}, 'module/root/test.js', []);"))));
   }
 
-  public void testImportPathDoesNotIncludeModuleRoot() {
+  // TODO(johnplaisted): This should strip module roots
+  public void testImportPathDoesIncludeModuleRoot() {
     moduleRoots = ImmutableList.of("module/root/");
 
     test(
@@ -500,10 +502,11 @@ public void testImportPathDoesNotIncludeModuleRoot() {
                 lines(
                     "$jscomp.registerAndLoadModule(function($$require, $$exports, $$module) {",
                     "  'test pragma';",
-                    "  var module$foo = $$require('foo.js');",
-                    "}, 'not/root/test.js', ['foo.js']);"))));
+                    "  var module$foo = $$require('module/root/foo.js');",
+                    "}, 'not/root/test.js', ['module/root/foo.js']);"))));
   }
 
+  // TODO(johnplaisted): This should respect different browser resolutions.
   public void testImportPathWithBrowserPrefixReplacementResolution() {
     resolutionMode = ModuleLoader.ResolutionMode.BROWSER_WITH_TRANSFORMED_PREFIXES;
     prefixReplacements = ImmutableMap.of("@root/", "");
@@ -516,7 +519,7 @@ public void testImportPathWithBrowserPrefixReplacementResolution() {
                 lines(
                     "$jscomp.registerAndLoadModule(function($$require, $$exports, $$module) {",
                     "  'test pragma';",
-                    "  var module$foo = $$require('foo.js');",
-                    "}, 'not/root/test.js', ['foo.js']);"))));
+                    "  var module$foo = $$require('@root/foo.js');",
+                    "}, 'not/root/test.js', ['@root/foo.js']);"))));
   }
 }

test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java

@@ -162,11 +162,11 @@ public void testEs6Module() throws IOException {
                 + "  }}, y:{enumerable:true, get:function() {\n"
                 + "    return module$nested$path$other.x;\n"
                 + "  }}});\n"
-                + "  var module$nested$path$other = $$require(\"nested/path/other.js\");\n"
+                + "  var module$nested$path$other = $$require(\"./other.js\");\n"
                 + "  var local;\n"
                 + "  function foo() {\n"
                 + "    return local;\n"
                 + "  }\n"
-                + "}, \"nested/path/foo.js\", [\"nested/path/other.js\"]);\n");
+                + "}, \"nested/path/foo.js\", [\"./other.js\"]);\n");
   }
 }