dojo / util

Dojo 1 - build utilities. Please submit bugs to https://bugs.dojotoolkit.org/
https://dojotoolkit.org/
Other
60 stars 105 forks source link

1.16.3 by default generates at least ECMAScript 2015, while it was ECMASCRIPT3 in 1.16.2 and earlier #81

Closed jandppw closed 3 years ago

jandppw commented 4 years ago

In 1.16.2 -> 1.16.3, the closure compiler was updated.

1.16.2

jand@Desjani:pictoperfect-buildenv>java -jar src/uiBuild/yymmdd-HHMM/lib/util/closureCompiler/compiler.jar --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20160911
Built on: 2016-09-13 16:51

1.16.3

jand@Desjani:pictoperfect-buildenv>java -jar src/uiBuild/yymmdd-HHMM/lib/util/closureCompiler/compiler.jar --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20200112
Built on: 2020-01-13 22:21

That's quite a jump, of over 3 years.

A new build, created with util util@1.16.3, fails on legacy infrastructure.

The reason is that the closure compiler outputs a more modern version of JavaScript. As an example, 1.16.3 outputs shorthand property names:

c.definitions = {
          Math,
          window,
          global: window,
          module: k.selfResolving(function(a, b) {
  …

where 1.16.2 does not:

root.definitions = {
…
        Math: Math,
        window: window,
        global: window,
        module: expression.selfResolving(function(mid, lazy){

As far as I know, shorthand property names are an ECMAScript 2015 feature.

From the closure compiler release notes, I learn that a command line property --language_out was added in release

The release notes for v20170806 say

August 6, 2017 (v20170806)

… ECMASCRIPT_2017 is now the default language mode. … The output language defaults to ES5 instead of to the input language. …

The release notes for v20170806 say

August 5, 2018 (v20180805)

You can now specify STABLE for input and/or output language level to request the latest version of JavaScript that is fully supported by the compiler. Currently, this means ES_2017 for input and ES5 for output.

I can find no other references to changes in the output language level in the release notes, so I need to assume that it is still (intended to be) ES5. So either this is wrong, or the dojo build system is actively requesting another output language level. I cannot find prove of this. Yet, experiments show that the new setup by default generates at least ECMASCRIPT_2015.

Workaround

The more recent closure compiler supports a --language_out command line property;

v20200112 --help says:

 --language_out VAL                     : Sets the language spec to which
                                          output should conform. Options:
                                          ECMASCRIPT3, ECMASCRIPT5, ECMASCRIPT5_
                                          STRICT, ECMASCRIPT_2015, ECMASCRIPT_20
                                          16, ECMASCRIPT_2017, ECMASCRIPT_2018,
                                          ECMASCRIPT_2019, STABLE

After much searching, I found out that we can add a optimizeOptions property to our build profile build.profile.js:

…
  mini: true,
  optimize: "closure",
  layerOptimize: "closure",
  optimizeOptions: {
    languageOut: "ECMASCRIPT3"
  },
  stripConsole: "normal",
…

that generates a working build.

languageOut: "ECMASCRIPT5" generated a build that does not work, so it seems that 1.16.2 -> 1.16.3 does a jump from ECMASCRIPT3 to ECMASCRIPT_2015 or later.

The use of optimizeOptions in the build profile is not documented. But, in our build process at least, it is copied into bc, which is included from util/build/buildControl.js, which is used in util/build/transforms/optimizer/sendJob.js which calls util/build/optimizeRunner.js, where it is used as command line argument in the runJava call.

Suggestion

This is quite a jump for a patch version 1.16.2 -> 1.16.3.

I suggest

dasa commented 4 years ago

Another good default for language_out could be NO_TRANSPILE. This is used here: https://github.com/ampproject/rollup-plugin-closure-compiler/blob/main/src/options.ts#L136

dasa commented 4 years ago

Another example of setting language_out to NO_TRANSPILE: https://github.com/emscripten-core/emscripten/blob/master/tools/building.py#L1055

msssk commented 4 years ago

@dasa thanks for the suggestion, this sounds reasonable.

@jandppw I think the default option should only be applied if the build profile has configured Google Closure Compiler as the optimizer. This logic has been implemented in #83. In my testing optimizeOptions is untouched if Closure is not configured or if optimizeOptions.languageOut is already set. If Closure is configured and optimizeOptions.languageOut is not configured then it will be set to 'NO_TRANSPILE'. Can you let me know if the changes in #83 work for your case?

jandppw commented 4 years ago

@msssk Thx for the fix. I reviewed, and it looks ok to me. I do suggest a tad of documentation somewhere.

In our case, given the ancient target, I am glad to keep an explicit ECMASCRIPT3 in there.

I would be glad to test, but I cannot guarantee I will find the time over the next few weeks. Work on this project is intermittent. If a new dojo 1 version is released with the patch when we return to the project, it will be used. I'll see what I can do in the mean time, but don't hold off on our account.

I'm curious: if a 1.17 would be released, what are the plans for this this default?

msssk commented 4 years ago

After more investigation and consideration I think given the importance of legacy support for Dojo we should probably strive to maintain the same behavior for builds. Prior to https://github.com/dojo/util/commit/86f00424a1ca5f7416c46bd778cb802418d60b8c Dojo's build behavior was:

The way to maintain this behavior is to provide explicit default settings (which can of course be configured in the build profile to different values):

{
    optimizeOptions: {
        languageIn: 'ECMASCRIPT3',
        languageOut: 'ECMASCRIPT3'
}

Unfortunately we can't completely provide the same behavior because the behavior of the Google Closure Compiler has changed. Prior to the introduction of block scope in ES6 Closure accepted function declarations within blocks without complaint. Once Closure added support for ES6 and block scope it became impractical to allow block-scoped functions in pre-ES6 code (see google/closure-compiler#3189).

In order for Dojo builds to work with the newer Closure releases and language set to 'ECMASCRIPT3' several modules that have block-scoped functions need to be converted to function expressions.

Application code that previously built fine will start seeing Closure compiler errors for block-scoped function use. There are two options to fix it:

dasa commented 4 years ago

Neither option sounds good. An alternative would be to revert the Closure upgrade, and add support for Terser as an alternative optimizer.

msssk commented 4 years ago

@dasa agreed, after thinking about and discussing this more our current inclination is to set the default build configuration as follows:

{
    optimizeOptions: {
        languageIn: 'ECMASCRIPT_2017',
        languageOut: 'ECMASCRIPT3'
    }
}

This allows existing Dojo builds that work to continue working. What is lost is the ability to receive errors from the build process about ES5+ code (if you are expecting all your source to be ES3). We will update Dojo's code to remove use of block-scoped functions so that developers can change the build configuration to languageIn: 'ECMASCRIPT3' and as long as their application code is ES3-compliant then the build will succeed.