diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java index 9b5fb199daf..90d9b102131 100644 --- a/src/com/google/javascript/jscomp/CommandLineRunner.java +++ b/src/com/google/javascript/jscomp/CommandLineRunner.java @@ -30,11 +30,14 @@ import com.google.common.io.Files; import com.google.javascript.jscomp.AbstractCommandLineRunner.CommandLineConfig.ErrorFormatOption; import com.google.javascript.jscomp.CompilerOptions.IsolationMode; +import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.SourceMap.LocationMapping; import com.google.javascript.jscomp.deps.ClosureBundler; import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.jscomp.parsing.parser.FeatureSet; +import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature; import com.google.javascript.jscomp.transpile.BaseTranspiler; +import com.google.javascript.jscomp.transpile.BaseTranspiler.CompilerSupplier; import com.google.javascript.jscomp.transpile.Transpiler; import com.google.javascript.rhino.TokenStream; import com.google.protobuf.TextFormat; @@ -1843,13 +1846,9 @@ protected Compiler createCompiler() { return new Compiler(getErrorPrintStream()); } - private ClosureBundler bundler; - - private ClosureBundler getBundler() { - if (bundler != null) { - return bundler; - } - + @Override + protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, String content) + throws IOException { ImmutableList moduleRoots; if (!flags.moduleRoot.isEmpty()) { moduleRoots = ImmutableList.copyOf(flags.moduleRoot); @@ -1858,25 +1857,29 @@ private ClosureBundler getBundler() { } CompilerOptions options = createOptions(); - return bundler = new ClosureBundler( - Transpiler.NULL, - new BaseTranspiler( - new BaseTranspiler.EsmToCjsCompilerSupplier( - options.getModuleResolutionMode(), - moduleRoots, - options.getBrowserResolverPrefixReplacements()), - /* runtimeLibraryName= */ "")); - } - @Override - protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, String content) - throws IOException { - getBundler().withPath(input.getName()).appendTo(out, input, content); + new ClosureBundler( + "es6".equals(input.getLoadFlags().get("module")) + ? new BaseTranspiler( + new CompilerSupplier( + LanguageMode.ECMASCRIPT_NEXT.toFeatureSet().without(Feature.MODULES), + options.getModuleResolutionMode(), + moduleRoots, + options.getBrowserResolverPrefixReplacements()), + /* runtimeLibraryName= */ "") + : Transpiler.NULL) + .withPath(input.getName()) + .appendTo(out, input, content); } @Override protected void appendRuntimeTo(Appendable out) throws IOException { - getBundler().appendRuntimeTo(out); + new ClosureBundler( + new BaseTranspiler( + new CompilerSupplier(), + // Default runtime is the module runtime. + /* runtimeLibraryName= */ "")) + .appendRuntimeTo(out); } @Override diff --git a/src/com/google/javascript/jscomp/deps/ClosureBundler.java b/src/com/google/javascript/jscomp/deps/ClosureBundler.java index e56f02c9576..1e8f58af697 100644 --- a/src/com/google/javascript/jscomp/deps/ClosureBundler.java +++ b/src/com/google/javascript/jscomp/deps/ClosureBundler.java @@ -51,7 +51,7 @@ public ClosureBundler() { } public ClosureBundler(Transpiler transpiler) { - this(transpiler, BaseTranspiler.ES_MODULE_TO_CJS_TRANSPILER); + this(transpiler, BaseTranspiler.LATEST_TRANSPILER); } public ClosureBundler(Transpiler transpiler, Transpiler es6ModuleTranspiler) { @@ -79,6 +79,20 @@ private ClosureBundler( this.es6ModuleTranspiler = es6ModuleTranspiler; } + public ClosureBundler withTranspilers( + Transpiler newTranspiler, Transpiler newEs6ModuleTranspiler) { + return new ClosureBundler( + newTranspiler, newEs6ModuleTranspiler, mode, sourceUrl, path, sourceMapCache); + } + + public ClosureBundler withTranspiler(Transpiler newTranspiler) { + return withTranspilers(newTranspiler, es6ModuleTranspiler); + } + + public ClosureBundler withEs6ModuleTranspiler(Transpiler newEs6ModuleTranspiler) { + return withTranspilers(transpiler, newEs6ModuleTranspiler); + } + public final ClosureBundler useEval(boolean useEval) { EvalMode newMode = useEval ? EvalMode.EVAL : EvalMode.NORMAL; return new ClosureBundler( diff --git a/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java b/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java index bd46ea22c35..5ba3087d338 100644 --- a/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java +++ b/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java @@ -27,7 +27,6 @@ import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.DiagnosticGroup; import com.google.javascript.jscomp.DiagnosticType; -import com.google.javascript.jscomp.Es6RewriteModulesToCommonJsModules; import com.google.javascript.jscomp.PropertyRenamingPolicy; import com.google.javascript.jscomp.Result; import com.google.javascript.jscomp.SourceFile; @@ -37,7 +36,8 @@ import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.jscomp.deps.ModuleLoader.PathEscaper; import com.google.javascript.jscomp.deps.ModuleLoader.ResolutionMode; -import com.google.javascript.rhino.Node; +import com.google.javascript.jscomp.parsing.parser.FeatureSet; +import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -74,11 +74,17 @@ public String runtime() { return sb.toString(); } - public static final BaseTranspiler ES5_TRANSPILER = new BaseTranspiler( - new CompilerSupplier(), "es6_runtime"); + public static final BaseTranspiler LATEST_TRANSPILER = to(FeatureSet.latest(), ""); - public static final BaseTranspiler ES_MODULE_TO_CJS_TRANSPILER = - new BaseTranspiler(new EsmToCjsCompilerSupplier(), ""); + public static final BaseTranspiler ES5_TRANSPILER = to(LanguageMode.ECMASCRIPT5.toFeatureSet()); + + public static final BaseTranspiler to(FeatureSet featureSet, String runtime) { + return new BaseTranspiler(new CompilerSupplier(featureSet), runtime); + } + + public static final BaseTranspiler to(FeatureSet featureSet) { + return to(featureSet, "es6_runtime"); + } /** * Wraps the Compiler into a more relevant interface, making it @@ -92,10 +98,19 @@ public static class CompilerSupplier { protected final ResolutionMode moduleResolution; protected final ImmutableList moduleRoots; protected final ImmutableMap prefixReplacements; + protected final FeatureSet outputFeatureSet; public CompilerSupplier() { + this(LanguageMode.ECMASCRIPT5.toFeatureSet()); + } + + public CompilerSupplier(FeatureSet outputFeatureSet) { // Use the default resolution mode - this(new CompilerOptions().getModuleResolutionMode(), ImmutableList.of(), ImmutableMap.of()); + this( + outputFeatureSet, + new CompilerOptions().getModuleResolutionMode(), + ImmutableList.of(), + ImmutableMap.of()); } /** @@ -106,9 +121,11 @@ public CompilerSupplier() { * ModuleLoader.ResolutionMode#BROWSER_WITH_TRANSFORMED_PREFIXES} */ public CompilerSupplier( + FeatureSet outputFeatureSet, ModuleLoader.ResolutionMode moduleResolution, ImmutableList moduleRoots, ImmutableMap prefixReplacements) { + this.outputFeatureSet = outputFeatureSet; this.moduleResolution = moduleResolution; this.moduleRoots = moduleRoots; this.prefixReplacements = prefixReplacements; @@ -157,16 +174,8 @@ protected CompilerOptions options() { protected void setOptions(CompilerOptions options) { options.setLanguageIn(LanguageMode.ECMASCRIPT_NEXT); - // TODO(sdh): It would be nice to allow people to output code in - // strict mode. But currently we swallow all the input language - // strictness checks, and there are various tests that are never - // compiled and so are broken when we output 'use strict'. We - // could consider adding some sort of logging/warning/error in - // cases where the input was not strict, though there could still - // be semantic differences even if syntax is strict. Possibly - // the first step would be to allow the option of outputting strict - // and then change the default and see what breaks. b/33005948 - options.setLanguageOut(LanguageMode.ECMASCRIPT5); + options.setOutputFeatureSet(outputFeatureSet.without(Feature.MODULES)); + options.setEmitUseStrict(false); options.setQuoteKeywordProperties(true); options.setSkipNonTranspilationPasses(true); options.setVariableRenaming(VariableRenamingPolicy.OFF); @@ -206,76 +215,6 @@ protected void setOptions(CompilerOptions options) { DiagnosticType.error("JSC_CANNOT_CONVERT", "")); } - /** - * CompilerSupplier that only transforms EcmaScript Modules into a form that can be safely - * transformed on a file by file basis and concatenated. - */ - public static class EsmToCjsCompilerSupplier extends CompilerSupplier { - - public EsmToCjsCompilerSupplier() { - super(); - } - - public EsmToCjsCompilerSupplier( - ModuleLoader.ResolutionMode moduleResolution, - ImmutableList moduleRoots, - ImmutableMap prefixReplacements) { - super(moduleResolution, moduleRoots, prefixReplacements); - } - - - @Override - public CompileResult compile(URI path, String code) { - CompilerOptions options = new CompilerOptions(); - options.setLanguageIn(LanguageMode.ECMASCRIPT_NEXT); - options.setEmitUseStrict(false); - options.setSourceMapOutputPath("/dev/null"); - options.setSourceMapIncludeSourcesContent(true); - options.setPrettyPrint(true); - options.setModuleResolutionMode(moduleResolution); - options.setModuleRoots(moduleRoots); - options.setBrowserResolverPrefixReplacements(prefixReplacements); - - // Create a compiler and run specifically this one pass on it. - Compiler compiler = compiler(); - compiler.init( - ImmutableList.of(), - ImmutableList.of(SourceFile.fromCode(path.toString(), code)), - options); - compiler.parseForCompilation(); - - boolean transpiled = false; - - if (!compiler.hasErrors() - && compiler.getRoot().getSecondChild().getFirstFirstChild().isModuleBody() - && !compiler - .getRoot() - .getSecondChild() - .getFirstChild() - .getBooleanProp(Node.GOOG_MODULE)) { - new Es6RewriteModulesToCommonJsModules(compiler) - .process(null, compiler.getRoot().getSecondChild()); - compiler.getRoot().getSecondChild().getFirstChild().putBooleanProp(Node.TRANSPILED, true); - transpiled = true; - } - - Result result = compiler.getResult(); - String source = compiler.toSource(); - StringBuilder sourceMap = new StringBuilder(); - if (result.sourceMap != null) { - try { - result.sourceMap.appendTo(sourceMap, path.toString()); - } catch (IOException e) { - // impossible, and not a big deal even if it did happen. - } - } - if (result.errors.length > 0) { - throw new TranspilationException(compiler, result.errors, result.warnings); - } - return new CompileResult(source, transpiled, transpiled ? sourceMap.toString() : ""); - } - } - /** * The source together with the additional compilation results. */ diff --git a/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java b/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java index 5639935feb6..05c37570b75 100644 --- a/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java +++ b/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java @@ -20,6 +20,8 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; +import com.google.javascript.jscomp.bundle.TranspilationException; +import com.google.javascript.jscomp.transpile.BaseTranspiler; import com.google.javascript.jscomp.transpile.TranspileResult; import com.google.javascript.jscomp.transpile.Transpiler; import java.io.IOException; @@ -140,9 +142,10 @@ public void testEs6Module() throws IOException { String input = "import {x} from './other.js';\n" + "export {x as y};" - + "var local;\n" + + "let local;\n" + "export function foo() { return local; }\n"; - ClosureBundler bundler = new ClosureBundler().withPath("nested/path/foo.js"); + ClosureBundler bundler = + new ClosureBundler(BaseTranspiler.LATEST_TRANSPILER).withPath("nested/path/foo.js"); StringBuilder sb = new StringBuilder(); bundler.appendRuntimeTo(sb); bundler.appendTo( @@ -163,10 +166,61 @@ public void testEs6Module() throws IOException { + " return module$nested$path$other.x;\n" + " }}});\n" + " var module$nested$path$other = $$require(\"nested/path/other.js\");\n" - + " var local;\n" + + " let local;\n" + " function foo() {\n" + " return local;\n" + " }\n" + "}, \"nested/path/foo.js\", [\"nested/path/other.js\"]);\n"); } + + public void testPassThroughIfNoTranspilationNeeded() throws Exception { + String input = "/** Hello Comments! */ const s = 0;\n let intended;"; + ClosureBundler bundler = new ClosureBundler(BaseTranspiler.LATEST_TRANSPILER); + StringBuilder sb = new StringBuilder(); + bundler.appendTo( + sb, + SimpleDependencyInfo.builder("", "").build(), + input); + assertThat(sb.toString()).isEqualTo(input); + } + + public void testCommentsAndFormattingRemovedWithTranspilation() throws Exception { + String input = "/** Hello Comments! */ const s = 0;\n let intended;"; + ClosureBundler bundler = new ClosureBundler(BaseTranspiler.ES5_TRANSPILER); + StringBuilder sb = new StringBuilder(); + bundler.appendTo( + sb, + SimpleDependencyInfo.builder("", "").build(), + input); + assertThat(sb.toString()).isEqualTo("var s = 0;\nvar intended;\n"); + } + + public void testFullWidthLowLineWithDefaultTranspilerIsOkay() throws Exception { + // The last character is something the compiler doesn't handle correctly + String input = "var ａｅｓｔｈｅｔｉｃ＿"; + ClosureBundler bundler = new ClosureBundler(); + StringBuilder sb = new StringBuilder(); + bundler.appendTo( + sb, + SimpleDependencyInfo.builder("", "").build(), + input); + assertThat(sb.toString()).isEqualTo(input); + } + + // TODO(johnplaisted): If / when the compiler can parse full width low line in identifiers + // this should be okay to be transpiled. + public void testFullWidthLowLineInTranspiledCodeIsError() throws Exception { + // The last character is something the compiler doesn't handle correctly + String input = "let ａｅｓｔｈｅｔｉｃ＿"; + ClosureBundler bundler = new ClosureBundler(BaseTranspiler.ES5_TRANSPILER); + StringBuilder sb = new StringBuilder(); + try { + bundler.appendTo(sb, SimpleDependencyInfo.builder("", "").build(), input); + fail("Expected an exception"); + } catch (TranspilationException e) { + assertThat(e) + .hasMessageThat() + .contains("Parse error. Character '＿' (U+FF3F) is not a valid identifier start char"); + } + } }