Make paths relative rather than absolute in the Es6RewriteImportPaths…

… pass.

Just resolving import paths doesn't work for all servers. If a client transpiles the paths will be rooted (starting with "/") but the path of the file could be considered to be absolute (with a scheme). So you'll end up with something like:

$jscomp.registerAndLoadModule(function($$require) {
}, "http://localhost:port/file.js", []);

$jscomp.registerAndLoadModule(function($$require) {
  $$require('/file.js');
}, "http://localhost:port/path.js", ['/file.js']);

Which won't ever be resolved as the jscomp module runtime's path resolution is very simple. Making these paths relative will resolve them correctly as the runtime can handle those (so in the above exampled '/file.js' becomes './file.js').

Ideally servers would just transpile the ES6 module themselves to avoid client transpilation, but this isn't always possible.

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

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

@@ -2753,9 +2753,9 @@ public enum Es6ModuleTranspilation {
     NONE,
 
     /**
-     * Rewrite import paths to resolved paths only.
+     * Rewrite import paths to resolved, relative paths only.
      */
-    RESOLVE_IMPORT_PATHS,
+    RELATIVIZE_IMPORT_PATHS,
 
     /**
      * Rewrite to common js like modules for bundling.

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

@@ -160,7 +160,7 @@ protected List<PassFactory> getTranspileOnlyPasses() {
         case TO_COMMON_JS_LIKE_MODULES:
           TranspilationPasses.addEs6ModuleToCjsPass(passes);
           break;
-        case RESOLVE_IMPORT_PATHS:
+        case RELATIVIZE_IMPORT_PATHS:
           TranspilationPasses.addEs6RewriteImportPathPass(passes);
           break;
         case NONE:

src/com/google/javascript/jscomp/Es6RewriteImportPaths.java → src/com/google/javascript/jscomp/Es6RelativizeImportPaths.java

@@ -16,20 +16,27 @@
 
 package com.google.javascript.jscomp;
 
+import com.google.common.annotations.GwtIncompatible;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback;
+import com.google.javascript.jscomp.deps.ModuleLoader;
 import com.google.javascript.rhino.Node;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Paths;
 
 /**
- * Rewrites ES6 import paths to be absolute according to the compiler's module resolver.
+ * Rewrites ES6 import paths to be relative after resolving according to the compiler's module
+ * resolver.
  *
- * <p>Useful for servers that wish to preserve ES6 modules, meaning their paths need to be valid
- * in the browser.</p>
+ * <p>Useful for servers that wish to preserve ES6 modules, meaning their paths need to be valid in
+ * the browser.
  */
-public class Es6RewriteImportPaths implements CompilerPass {
+@GwtIncompatible("java.net.URI")
+public class Es6RelativizeImportPaths implements CompilerPass {
 
   private final AbstractCompiler compiler;
 
-  public Es6RewriteImportPaths(AbstractCompiler compiler) {
+  public Es6RelativizeImportPaths(AbstractCompiler compiler) {
     this.compiler = compiler;
   }
 
@@ -65,7 +72,50 @@ public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent)
     private void visitImport(NodeTraversal t, Node importDecl) {
       Node specifierNode = importDecl.getLastChild();
       String specifier = specifierNode.getString();
+
+      // Leave relative and truly absolute paths (those with a scheme) alone. Only transform
+      // absolute paths without a scheme (starting with "/") and ambiguous paths.
+      if (ModuleLoader.isRelativeIdentifier(specifier)) {
+        return;
+      } else {
+        try {
+          URI specifierURI = new URI(specifier);
+          if (specifierURI.isAbsolute()) {
+            return;
+          }
+        } catch (URISyntaxException e) {
+          return;
+        }
+      }
+
+      String scriptPath = t.getInput().getPath().toString();
+
+      try {
+        // If the script path has a scheme / host / port then just use the path part.
+        scriptPath = new URI(scriptPath).getPath();
+      } catch (URISyntaxException e) {
+        return;
+      }
+
       String newSpecifier = t.getInput().getPath().resolveModuleAsPath(specifier).toString();
+
+      // If a module root is stripped then this won't start with "/" when it probably should.
+      if (!newSpecifier.startsWith("/")) {
+        newSpecifier = "/" + newSpecifier;
+      }
+
+      newSpecifier =
+          Paths.get(scriptPath)
+              .getParent()
+              .relativize(Paths.get(newSpecifier))
+              .toString();
+
+      // Relativizing two paths with the same directory yields an ambiguous path rather than one
+      // starting with "./".
+      if (ModuleLoader.isAmbiguousIdentifier(newSpecifier)) {
+        newSpecifier = "./" + newSpecifier;
+      }
+
       if (!newSpecifier.equals(specifier)) {
         specifierNode.setString(newSpecifier);
         t.reportCodeChange(specifierNode);

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

@@ -127,7 +127,7 @@ public static void addEs6ModuleToCjsPass(List<PassFactory> passes) {
   }
 
   public static void addEs6RewriteImportPathPass(List<PassFactory> passes) {
-    passes.add(es6RewriteImportPaths);
+    passes.add(es6RelativizeImportPaths);
   }
 
   /** Deprecated: use addPostCheckTranspilationPasses instead. */
@@ -170,11 +170,11 @@ protected FeatureSet featureSet() {
       };
 
   /** Rewrites ES6 modules import paths to be browser compliant */
-  private static final PassFactory es6RewriteImportPaths =
-      new PassFactory("es6RewriteImportPaths", true) {
+  private static final PassFactory es6RelativizeImportPaths =
+      new PassFactory("es6RelativizeImportPaths", true) {
         @Override
         protected CompilerPass create(AbstractCompiler compiler) {
-          return new Es6RewriteImportPaths(compiler);
+          return new Es6RelativizeImportPaths(compiler);
         }
 
         @Override

src/com/google/javascript/jscomp/gwt/super/com/google/javascript/jscomp/Es6RelativizeImportPaths.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2018 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 com.google.javascript.rhino.Node;
+
+/** GWT compatible no-op replacement for {@code Es6RelativizeImportPaths} */
+class Es6RelativizeImportPaths implements CompilerPass {
+  Es6RelativizeImportPaths(AbstractCompiler compiler) {
+  }
+
+  @Override
+  public void process(Node externs, Node root) {
+    throw new UnsupportedOperationException("Es6RelativizeImportPaths not implemented");
+  }
+}

test/com/google/javascript/jscomp/Es6RewriteImportPathsTest.java → test/com/google/javascript/jscomp/Es6RelativizeImportPathsTest.java

@@ -18,11 +18,12 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.javascript.jscomp.deps.ModuleLoader.PathEscaper;
 import com.google.javascript.jscomp.deps.ModuleLoader.ResolutionMode;
 import java.util.List;
 
 
-public class Es6RewriteImportPathsTest extends CompilerTestCase {
+public class Es6RelativizeImportPathsTest extends CompilerTestCase {
 
   private ImmutableMap<String, String> prefixReplacements;
   private ResolutionMode moduleResolutionMode;
@@ -42,76 +43,126 @@ protected CompilerOptions getOptions() {
     options.setBrowserResolverPrefixReplacements(prefixReplacements);
     options.setModuleResolutionMode(moduleResolutionMode);
     options.setModuleRoots(moduleRoots);
+    options.setPathEscaper(PathEscaper.CANONICALIZE_ONLY);
     return options;
   }
 
   @Override
   protected CompilerPass getProcessor(Compiler compiler) {
-    return new Es6RewriteImportPaths(compiler);
+    return new Es6RelativizeImportPaths(compiler);
   }
 
-  public void testRelativePath() {
-    test(
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import './foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/foo.js';")));
+  public void testLocalRelativePathStayTheSame() {
+    testSame(ImmutableList.of(SourceFile.fromCode("/file.js", "import './foo.js';")));
 
-    test(
-        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import './foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '/nested/foo.js';")));
+    testSame(ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import './foo.js';")));
 
-    test(
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import './nested/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/nested/foo.js';")));
+    testSame(ImmutableList.of(SourceFile.fromCode("/file.js", "import './nested/foo.js';")));
+
+    testSame(
+        ImmutableList.of(SourceFile.fromCode("http://google.com/file.js", "import './foo.js';")));
+  }
+
+  public void testRelativeParentPathStayTheSame() {
+    testSame(ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../foo.js';")));
+
+    testSame(
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../nested/foo.js';")));
+
+    testSame(ImmutableList.of(SourceFile.fromCode("/p0/p1/file.js", "import '../p2/foo.js';")));
+
+    testSame(ImmutableList.of(SourceFile.fromCode("/p0/p1/file.js", "import '../../foo.js';")));
+
+    testSame(
+        ImmutableList.of(
+            SourceFile.fromCode("http://google.com/nested/file.js", "import '../foo.js';")));
   }
 
-  public void testRelativeParentPath() {
+  public void testAbsolutePathsStayTheSame() {
+    // Absolute means that with a scheme, not those starting with "/"
+    testSame(
+        ImmutableList.of(
+            SourceFile.fromCode(
+                "http://google.com/file.js", "import 'http://google.com/other/file.js';")));
+
+    testSame(
+        ImmutableList.of(
+            SourceFile.fromCode(
+                "http://golang.org/file.js", "import 'http://google.com/other/file.js';")));
+
+    testSame(
+        ImmutableList.of(
+            SourceFile.fromCode(
+                "/file.js", "import 'http://google.com/other/file.js';")));
+  }
+
+  public void testRootedPathsBecomeRelative() {
     test(
-        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './foo.js';")));
 
     test(
-        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../nested/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '/nested/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../foo.js';")));
 
     test(
-        ImmutableList.of(SourceFile.fromCode("/p0/p1/file.js", "import '../p2/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/p0/p1/file.js", "import '/p0/p2/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '/path/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../path/foo.js';")));
 
     test(
-        ImmutableList.of(SourceFile.fromCode("/p0/p1/file.js", "import '../../foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/p0/p1/file.js", "import '/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/nested/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './nested/foo.js';")));
   }
 
-  public void testNonStandardResolution() {
+  public void testNonStandardResolutionBecomesRelative() {
     moduleResolutionMode = ResolutionMode.BROWSER_WITH_TRANSFORMED_PREFIXES;
     prefixReplacements = ImmutableMap.of("raw0/", "/mapped0/", "raw1/", "/mapped1/");
 
     test(
         ImmutableList.of(SourceFile.fromCode("/file.js", "import 'raw0/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/mapped0/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './mapped0/foo.js';")));
+
+    test(
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import 'raw0/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../mapped0/foo.js';")));
 
     test(
         ImmutableList.of(SourceFile.fromCode("/file.js", "import 'raw0/bar/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/mapped0/bar/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './mapped0/bar/foo.js';")));
 
     test(
         ImmutableList.of(SourceFile.fromCode("/file.js", "import 'raw1/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/mapped1/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './mapped1/foo.js';")));
+
+    test(
+        ImmutableList.of(SourceFile.fromCode("/mapped1/file.js", "import 'raw1/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/mapped1/file.js", "import './foo.js';")));
+
+    test(
+        ImmutableList.of(SourceFile.fromCode("http://google.com/file.js", "import 'raw1/foo.js';")),
+        ImmutableList.of(
+            SourceFile.fromCode("http://google.com/file.js", "import './mapped1/foo.js';")));
+
+    test(
+        ImmutableList.of(
+            SourceFile.fromCode("http://google.com/nested/file.js", "import 'raw1/foo.js';")),
+        ImmutableList.of(
+            SourceFile.fromCode("http://google.com/file.js", "import '../mapped1/foo.js';")));
   }
 
-  public void testModuleRootsAreRemoved() {
+  public void testModuleRootsBecomeRelative() {
     moduleRoots = ImmutableList.of("root1", "root2");
 
     test(
         ImmutableList.of(SourceFile.fromCode("/file.js", "import '/root1/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import 'foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './foo.js';")));
 
     test(
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import '/root2/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import 'foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '/root2/foo.js';")),
+        ImmutableList.of(SourceFile.fromCode("/nested/file.js", "import '../foo.js';")));
 
     test(
         ImmutableList.of(SourceFile.fromCode("/file.js", "import '/root1/bar/foo.js';")),
-        ImmutableList.of(SourceFile.fromCode("/file.js", "import 'bar/foo.js';")));
+        ImmutableList.of(SourceFile.fromCode("/file.js", "import './bar/foo.js';")));
   }
 }