From d61cca977bc334537246041bf03326a950709eea Mon Sep 17 00:00:00 2001 From: johnplaisted Date: Wed, 27 Jun 2018 12:03:25 -0700 Subject: [PATCH] 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 --- .../javascript/jscomp/CompilerOptions.java | 4 +- .../javascript/jscomp/DefaultPassConfig.java | 2 +- ...ths.java => Es6RelativizeImportPaths.java} | 60 +++++++++- .../jscomp/TranspilationPasses.java | 8 +- .../jscomp/Es6RelativizeImportPaths.java | 30 +++++ ...java => Es6RelativizeImportPathsTest.java} | 111 +++++++++++++----- 6 files changed, 173 insertions(+), 42 deletions(-) rename src/com/google/javascript/jscomp/{Es6RewriteImportPaths.java => Es6RelativizeImportPaths.java} (55%) create mode 100644 src/com/google/javascript/jscomp/gwt/super/com/google/javascript/jscomp/Es6RelativizeImportPaths.java rename test/com/google/javascript/jscomp/{Es6RewriteImportPathsTest.java => Es6RelativizeImportPathsTest.java} (50%) diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java index 58b3e8769ae..8b7bd1d3578 100644 --- a/src/com/google/javascript/jscomp/CompilerOptions.java +++ b/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. diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index 0cc4360d0b6..f0f0f256232 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -160,7 +160,7 @@ protected List 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: diff --git a/src/com/google/javascript/jscomp/Es6RewriteImportPaths.java b/src/com/google/javascript/jscomp/Es6RelativizeImportPaths.java similarity index 55% rename from src/com/google/javascript/jscomp/Es6RewriteImportPaths.java rename to src/com/google/javascript/jscomp/Es6RelativizeImportPaths.java index 70c7dc169c0..a2caaf6075a 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteImportPaths.java +++ b/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. * - *

Useful for servers that wish to preserve ES6 modules, meaning their paths need to be valid - * in the browser.

+ *

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); diff --git a/src/com/google/javascript/jscomp/TranspilationPasses.java b/src/com/google/javascript/jscomp/TranspilationPasses.java index bc8cb6a5b9c..73cde10e031 100644 --- a/src/com/google/javascript/jscomp/TranspilationPasses.java +++ b/src/com/google/javascript/jscomp/TranspilationPasses.java @@ -127,7 +127,7 @@ public static void addEs6ModuleToCjsPass(List passes) { } public static void addEs6RewriteImportPathPass(List 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 diff --git a/src/com/google/javascript/jscomp/gwt/super/com/google/javascript/jscomp/Es6RelativizeImportPaths.java b/src/com/google/javascript/jscomp/gwt/super/com/google/javascript/jscomp/Es6RelativizeImportPaths.java new file mode 100644 index 00000000000..f295cb72ce8 --- /dev/null +++ b/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"); + } +} diff --git a/test/com/google/javascript/jscomp/Es6RewriteImportPathsTest.java b/test/com/google/javascript/jscomp/Es6RelativizeImportPathsTest.java similarity index 50% rename from test/com/google/javascript/jscomp/Es6RewriteImportPathsTest.java rename to test/com/google/javascript/jscomp/Es6RelativizeImportPathsTest.java index c21dea38617..9169557f268 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteImportPathsTest.java +++ b/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 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';"))); } }