From 3441ce33197fe96e43193666b3a5f43d5f52f328 Mon Sep 17 00:00:00 2001 From: johnplaisted Date: Thu, 14 Jun 2018 12:56:23 -0700 Subject: [PATCH] Automated g4 rollback of changelist 200580519. *** Reason for rollback *** Broke test runner tests (they don't transpile ES6 modules when bundling now) *** Original change description *** Allow the transpiler to be configured with a feature set output rather than to ES5. Allow ClosureBundler to be configured later with a different transpiler. When bundling only remove modules using the transpiler. *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=200603814 --- .../javascript/jscomp/CommandLineRunner.java | 45 ++++---- .../jscomp/deps/ClosureBundler.java | 32 ++++-- .../jscomp/transpile/BaseTranspiler.java | 107 ++++++++++++++---- .../jscomp/deps/ClosureBundlerTest.java | 4 +- 4 files changed, 133 insertions(+), 55 deletions(-) diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java index 90d9b102131..9b5fb199daf 100644 --- a/src/com/google/javascript/jscomp/CommandLineRunner.java +++ b/src/com/google/javascript/jscomp/CommandLineRunner.java @@ -30,14 +30,11 @@ 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; @@ -1846,9 +1843,13 @@ protected Compiler createCompiler() { return new Compiler(getErrorPrintStream()); } - @Override - protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, String content) - throws IOException { + private ClosureBundler bundler; + + private ClosureBundler getBundler() { + if (bundler != null) { + return bundler; + } + ImmutableList moduleRoots; if (!flags.moduleRoot.isEmpty()) { moduleRoots = ImmutableList.copyOf(flags.moduleRoot); @@ -1857,29 +1858,25 @@ protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, Str } CompilerOptions options = createOptions(); + return bundler = new ClosureBundler( + Transpiler.NULL, + new BaseTranspiler( + new BaseTranspiler.EsmToCjsCompilerSupplier( + options.getModuleResolutionMode(), + moduleRoots, + options.getBrowserResolverPrefixReplacements()), + /* runtimeLibraryName= */ "")); + } - 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 prepForBundleAndAppendTo(Appendable out, CompilerInput input, String content) + throws IOException { + getBundler().withPath(input.getName()).appendTo(out, input, content); } @Override protected void appendRuntimeTo(Appendable out) throws IOException { - new ClosureBundler( - new BaseTranspiler( - new CompilerSupplier(), - // Default runtime is the module runtime. - /* runtimeLibraryName= */ "")) - .appendRuntimeTo(out); + getBundler().appendRuntimeTo(out); } @Override diff --git a/src/com/google/javascript/jscomp/deps/ClosureBundler.java b/src/com/google/javascript/jscomp/deps/ClosureBundler.java index 18df33abefc..e56f02c9576 100644 --- a/src/com/google/javascript/jscomp/deps/ClosureBundler.java +++ b/src/com/google/javascript/jscomp/deps/ClosureBundler.java @@ -18,6 +18,7 @@ import com.google.common.base.Strings; import com.google.common.io.CharSource; import com.google.common.io.Files; +import com.google.javascript.jscomp.transpile.BaseTranspiler; import com.google.javascript.jscomp.transpile.TranspileResult; import com.google.javascript.jscomp.transpile.Transpiler; import java.io.File; @@ -34,6 +35,7 @@ public final class ClosureBundler { private final Transpiler transpiler; + private final Transpiler es6ModuleTranspiler; private final EvalMode mode; private final String sourceUrl; @@ -49,8 +51,13 @@ public ClosureBundler() { } public ClosureBundler(Transpiler transpiler) { + this(transpiler, BaseTranspiler.ES_MODULE_TO_CJS_TRANSPILER); + } + + public ClosureBundler(Transpiler transpiler, Transpiler es6ModuleTranspiler) { this( transpiler, + es6ModuleTranspiler, EvalMode.NORMAL, /* sourceUrl= */ null, /* path= */ "unknown_source", @@ -59,6 +66,7 @@ public ClosureBundler(Transpiler transpiler) { private ClosureBundler( Transpiler transpiler, + Transpiler es6ModuleTranspiler, EvalMode mode, String sourceUrl, String path, @@ -68,27 +76,23 @@ private ClosureBundler( this.sourceUrl = sourceUrl; this.path = path; this.sourceMapCache = sourceMapCache; - } - - public final ClosureBundler withTranspiler(Transpiler newTranspiler) { - return new ClosureBundler( - newTranspiler, mode, sourceUrl, path, sourceMapCache); + this.es6ModuleTranspiler = es6ModuleTranspiler; } public final ClosureBundler useEval(boolean useEval) { EvalMode newMode = useEval ? EvalMode.EVAL : EvalMode.NORMAL; return new ClosureBundler( - transpiler, newMode, sourceUrl, path, sourceMapCache); + transpiler, es6ModuleTranspiler, newMode, sourceUrl, path, sourceMapCache); } public final ClosureBundler withSourceUrl(String newSourceUrl) { return new ClosureBundler( - transpiler, mode, newSourceUrl, path, sourceMapCache); + transpiler, es6ModuleTranspiler, mode, newSourceUrl, path, sourceMapCache); } public final ClosureBundler withPath(String newPath) { return new ClosureBundler( - transpiler, mode, sourceUrl, newPath, sourceMapCache); + transpiler, es6ModuleTranspiler, mode, sourceUrl, newPath, sourceMapCache); } /** Append the contents of the string to the supplied appendable. */ @@ -122,6 +126,11 @@ public void appendTo( CharSource content) throws IOException { if (info.isModule()) { mode.appendGoogModule(transpile(content.read()), out, sourceUrl); + } else if ("es6".equals(info.getLoadFlags().get("module")) && transpiler == Transpiler.NULL) { + // TODO(johnplaisted): Make the default transpiler the ES_MODULE_TO_CJS_TRANSPILER. Currently + // some code is passing in unicode identifiers in non-ES6 modules the compiler fails to parse. + // Once this compiler bug is fixed we can always transpile. + mode.appendTraditional(transpileEs6Module(content.read()), out, sourceUrl); } else { mode.appendTraditional(transpile(content.read()), out, sourceUrl); } @@ -132,6 +141,9 @@ public void appendRuntimeTo(Appendable out) throws IOException { if (!runtime.isEmpty()) { mode.appendTraditional(runtime, out, null); } + if (transpiler == Transpiler.NULL) { + mode.appendTraditional(es6ModuleTranspiler.runtime(), out, null); + } } /** @@ -157,6 +169,10 @@ private String transpile(String s) { return transpile(s, transpiler); } + private String transpileEs6Module(String s) { + return transpile(s, es6ModuleTranspiler); + } + private enum EvalMode { EVAL { @Override diff --git a/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java b/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java index 694b1222c99..bd46ea22c35 100644 --- a/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java +++ b/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java @@ -27,6 +27,7 @@ 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; @@ -36,8 +37,7 @@ 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.jscomp.parsing.parser.FeatureSet; -import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature; +import com.google.javascript.rhino.Node; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -74,11 +74,11 @@ public String runtime() { return sb.toString(); } - public static final BaseTranspiler ES5_TRANSPILER = toLanguageMode(LanguageMode.ECMASCRIPT5); + public static final BaseTranspiler ES5_TRANSPILER = new BaseTranspiler( + new CompilerSupplier(), "es6_runtime"); - public static final BaseTranspiler toLanguageMode(LanguageMode languageOut) { - return new BaseTranspiler(new CompilerSupplier(languageOut.toFeatureSet()), "es6_runtime"); - } + public static final BaseTranspiler ES_MODULE_TO_CJS_TRANSPILER = + new BaseTranspiler(new EsmToCjsCompilerSupplier(), ""); /** * Wraps the Compiler into a more relevant interface, making it @@ -92,19 +92,10 @@ 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( - outputFeatureSet, - new CompilerOptions().getModuleResolutionMode(), - ImmutableList.of(), - ImmutableMap.of()); + this(new CompilerOptions().getModuleResolutionMode(), ImmutableList.of(), ImmutableMap.of()); } /** @@ -115,11 +106,9 @@ public CompilerSupplier(FeatureSet outputFeatureSet) { * 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; @@ -168,8 +157,16 @@ protected CompilerOptions options() { protected void setOptions(CompilerOptions options) { options.setLanguageIn(LanguageMode.ECMASCRIPT_NEXT); - options.setOutputFeatureSet(outputFeatureSet.without(Feature.MODULES)); - options.setEmitUseStrict(false); + // 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.setQuoteKeywordProperties(true); options.setSkipNonTranspilationPasses(true); options.setVariableRenaming(VariableRenamingPolicy.OFF); @@ -209,6 +206,76 @@ 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 03558762443..5639935feb6 100644 --- a/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java +++ b/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java @@ -20,7 +20,6 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; -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; @@ -143,8 +142,7 @@ public void testEs6Module() throws IOException { + "export {x as y};" + "var local;\n" + "export function foo() { return local; }\n"; - ClosureBundler bundler = - new ClosureBundler(BaseTranspiler.ES5_TRANSPILER).withPath("nested/path/foo.js"); + ClosureBundler bundler = new ClosureBundler().withPath("nested/path/foo.js"); StringBuilder sb = new StringBuilder(); bundler.appendRuntimeTo(sb); bundler.appendTo(