angular-cli: Third party ES2015 library causes build to hang/fail/be broken

sorry, maybe this is a stupid question. But has anyone actually been able to transpile es2015 in a lib only using the angular-cli?

angular-cli is working with default Terser parse options

I was mistaken. Terser defaults to using parse: { ecma: 8 } to parse trailing commas in function calls. angular-cli uses ecma 5 or 6 as a global override for all parse/compress/~mangle~/output sub-options. They ought to be set individually.

$ echo 'console.log(1, 2, 3,);' | node-v10.4.1 
1 2 3

$ echo 'console.log(1, 2, 3,);' | terser --ecma 8
console.log(1,2,3);

$ echo 'console.log(1, 2, 3,);' | terser --ecma 6
Parse error at 0:1,20
console.log(1, 2, 3,);
                    ^
ERROR: Unexpected token: punc ())

$ echo 'console.log(1, 2, 3,);' | terser --ecma 6 --parse ecma=8
console.log(1,2,3);

ng build --prod --buildOptimizer=false output an error

ERROR in main.abfced9f8456db47574e.js from Terser Unexpected token: punc ()) [main.abfced9f8456db47574e.js:188444,4]

Terser is encountering ES that is either invalid or it doesn’t know how to parse.

Run this command which presumably disables minification but still runs webpack in production mode:

ng build --prod --buildOptimizer=false --optimization=false <-- This completes

which ought to yield the same input that would have been sent to Terser in the previous failing command. (Note: if webpack is run in development mode it will not yield the same input and may not contain the error.)

You can then manually run each generated js file through the terser CLI for a more detailed error in context. For example:

terser main.abfced9f8456db47574e.js >/dev/null && echo SUCCESS || echo FAIL

Then put that failing generated file into a gist and post the link here.

For me, updating the dependency caused the problem worked, but the terser options builder should work as well

Hi @martin31821 The real solution should be from the cli to use terser default configuration. However in angular 7 and 8 you can create a custom builder (unnecessary complex solution for this scenario) That said you can try: https://github.com/keenondrums/angular-builder-custom-terser-options Or https://alligator.io/angular/custom-webpack-config/

The first solution doesn’t have support for angular 8 but the they are testing the next build to release it, you can try it from the branch check on the opened issues. The second possible solution, I didn’t tried it yet

I’m not sure adding a --no-minify is a good approach. That’s essentially a flag for debugging the build system itself.

@filipesilva Exactly - and that’s a good thing. Hours are spent analyzing these types of issues to isolate the problem when a --no-minify flag could shorten that time to minutes. This particular ticket wasn’t related to minification, but it could have quickly ruled it out as the problem. When an actual minifier bug does arise, it’s usually just the unminified production input that is required to debug the issue. That is not easy for users to produce.

Anyway, I’m not an Angular user. It’s your call.

Apparently there were quite a few other ES6 issues in the library, but Terser was only complaining about that one

As you’ve discovered, the ecma version parse checks in Terser are not comprehensive. When that particular trailing commas for call arguments feature was added the idea was to eventually add strict version checking for all language features. But it wasn’t a high priority.

Terser does have the ability to use Acorn as a parser for ES5 only. Acorn has strict parse checks for ecma versions. ESTree support for ES6+ is a work in progress in Terser.

@RyanHow I have to start by saying that we don’t provide support for custom configurations.

You might feel like just changing the node settings in webpack is a very small change, but config changes in webpack are never really isolated. Changing one part of the configuration can have drastic effects in other parts.

We don’t test for custom configurations and our stance is that once you use a custom configuration it really is up to you to make it work. In this reproduction, removing the custom configuration causes the whole build to fail.

Build Optimizer will spend an inordinate amount of time in the node_modules/composer-concerto/lib/introspect/parser.js file, yes. I agree with @clydin that it’s probably because it is a very big file and TypeScript takes a very long time to parse it since we use TS to parse all files. We’ve had problems like that in the past.

This probably needs a report to the typescript issue tracker, but the way we use TypeScript to parse every file is rather non-standard so I’m not sure something will come of it.

You can see TypeScript hanging on that file by running npx tsc --allowJs --outDir out-tsc node_modules/composer-concerto/lib/introspect/parser.js.

Similar problems are https://github.com/angular/angular-cli/issues/12153 and https://github.com/Microsoft/TypeScript/issues/17033.

@kzc I’m not sure adding a --no-minify is a good approach. That’s essentially a flag for debugging the build system itself. We try to stay clear of those because of two reasons: we already have way too many flags, and even if it was there you’d need a lot more functionality to have a fighting chance at accurately debugging the problem.

At that point it’s better to just go debug builds by fiddling with the webpack configs in node_modules/@angular-devkit/build-angular/src/angular-cli-files/webpack-configs/. It’s non-trivial to do so but it wouldn’t really be easier to try and figure out the right combination of build flags that would provide actionable information.

I’m not sure I follow the argument for setting the ecma options for parse, compress and output separately. It’s true that it would avoid the particular error in this issue, where a library has a trailing command in a function call (console.log(1, 2, 3,);).

But that doesn’t change the fact that the build targets ECMA5 and that’s invalid code in ECMA5. So I do think the build should fail. Code packaged in this way should only be used on builds targeting ECMA 8 or above, and the library should mention that.

On the CLI side we infer the target ECMA version from the tsconfig.json. This isn’t very obvious. We don’t even support anything but ECMA5 and 6. So this should be clearer as well and allow for things like ECMA8 and generally ESNEXT.

It’s not easy to figure out where that invalid code is though. I did it by doing ng build and then cat dist/angular-cicero-build/vendor.js | npx terser --ecma 6. It’s in this code block:

let logger = winston.createLogger({
    format: winston.format.combine(
        winston.format.json(),
        enumerateErrorFormat(),
        winston.format.colorize(),
        jsonColor(),  // <---- here
    ),
    transports: [
        new winston.transports.Console({
            level: 'info',
        }),
    ]
});

This code block belongs to ./node_modules/@accordproject/cicero-core/lib/logger.js.

I think what the CLI should do in these cases is actually verify these things in advance and provide decent actionable information. Something like

You are targeting ES5 but the "./node_modules/@accordproject/cicero-core/lib/logger.js" file contains invalid syntax at line 123:

        winston.format.colorize(),
        jsonColor(),  // <---- here
    ),

Please use a ES5 compatible version of this library or change your ES target.

We’ve been seeing more and more packaging/target ES problems as time goes by so this is definitely a growing problem.