Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Third party ES2015 library causes build to hang/fail/be broken #12975

Closed
RyanHow opened this issue Nov 16, 2018 · 28 comments
Closed

Third party ES2015 library causes build to hang/fail/be broken #12975

RyanHow opened this issue Nov 16, 2018 · 28 comments

Comments

@RyanHow
Copy link

RyanHow commented Nov 16, 2018

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Command (mark with an x)

- [ ] new
- [x] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [ ] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Versions

node --version
v8.11.3

npm --version
6.4.1

ng --version
Angular CLI: 7.0.6
Node: 8.11.3
OS: win32 x64
Angular: 7.0.4
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.10.6
@angular-devkit/build-angular     0.10.6
@angular-devkit/build-optimizer   0.10.6
@angular-devkit/build-webpack     0.10.6
@angular-devkit/core              7.0.6
@angular-devkit/schematics        7.0.6
@angular/cli                      7.0.6
@ngtools/webpack                  7.0.6
@schematics/angular               7.0.6
@schematics/update                0.10.6
rxjs                              6.3.3
typescript                        3.1.6
webpack                           4.19.1

Repro steps

git clone https://github.com/RyanHow/angular-cicero-build.git
cd angular-cicero-build
npm install
ng build --prod    <--- This hangs indefinitely
ng build --prod --buildOptimizer=false    <--- This fails
ng build --prod --buildOptimizer=false --optimization=false     <-- This completes

The above is a minimal repo imporing the @accordproject/cicero-core library.
It requires some custom webpack config. So it has @angular-builders/custom-webpack installed with

module.exports = {
    node: {
        fs: 'empty',
        net: 'empty',
        tls: 'empty'
    }
};

The log given by the failure

ng build --prod hangs indefinitely

45% building modules 297/298 modules 1 active ...poser-concerto\lib\introspect\parser.js

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

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

Desired functionality

I'd like to use the build optimization and optimizer. The bundle sizes are huge without them.
A workaround would be to exclude this library from optimization, or better, from specific forms of optimization causing the issue.

I've successfully used Terser in a react project importing this library, but I don't know what Angular CLI is doing under the hood to trigger the issue. So it is difficult to know where the issue is and what specific options are causing it. Verbose output doesn't really give any more info.

Mention any other details that might be useful

This is a repo with a minimal project
https://github.com/RyanHow/angular-cicero-build

@kzc
Copy link

kzc commented Nov 17, 2018

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.

@RyanHow
Copy link
Author

RyanHow commented Nov 18, 2018

Thanks @kzc

Terser completes successfully. Is there a way to see what options angular-cli is running it with?

@RyanHow
Copy link
Author

RyanHow commented Nov 18, 2018

I've actually managed to work around the terser issue by using a custom builder, extending the webpack config and replacing the optimization using https://github.com/meltedspark/angular-builders

It would be good to see what options angular is running it with so I can tweak it.

@kzc
Copy link

kzc commented Nov 18, 2018

Terser completes successfully

That's unfortunate. It indicates that angular or webpack did not produce the same JS which caused the Terser parse error in production mode.

@RyanHow
Copy link
Author

RyanHow commented Nov 18, 2018

That's unfortunate. It indicates that angular or webpack did not produce the same JS which caused the Terser parse error in production mode.

I think it is the options that angular is sending to Terser.

@kzc
Copy link

kzc commented Nov 18, 2018

It's a parse error. angular-cli is working with default Terser parse options - the other Terser options don't matter because it's already failed by that point.

const terserOptions = {
ecma: wco.supportES2015 ? 6 : 5,
warnings: !!buildOptions.verbose,
safari10: true,
output: {
ascii_only: true,
comments: false,
webkit: true,
},
// On server, we don't want to compress anything. We still set the ngDevMode = false for it
// to remove dev code.
compress: (buildOptions.platform == 'server' ? {
global_defs: {
ngDevMode: false,
},
} : {
pure_getters: buildOptions.buildOptimizer,
// PURE comments work best with 3 passes.
// See https://github.com/webpack/webpack/issues/2899#issuecomment-317425926.
passes: buildOptions.buildOptimizer ? 3 : 1,
global_defs: {
ngDevMode: false,
},
}),
// We also want to avoid mangling on server.
...(buildOptions.platform == 'server' ? { mangle: false } : {}),
};

You have to obtain the unminified production code that webpack passes to Terser in the failing case.

@kzc
Copy link

kzc commented Nov 18, 2018

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);

@kzc
Copy link

kzc commented Nov 18, 2018

Run

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

and post line 188444 from the file:

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

@RyanHow
Copy link
Author

RyanHow commented Nov 18, 2018

Thanks! I've found the issue.

The output from angular pre-Terser with optimization=false is different. So I ran terser with --ecma=6 on the unminified output and found a trailing comma in a function call.

Thanks for your help debugging.

Now just to get the build optimizer working :)

@RyanHow
Copy link
Author

RyanHow commented Nov 18, 2018

This may narrow down the build optimizer issue

If you clone the repo in the issue, then run the following command

./node_modules/.bin/build-optimizer node_modules/composer-concerto/lib/introspect/parser.js

It hangs indefinitely

@clydin
Copy link
Member

clydin commented Nov 18, 2018

That appears to be a typescript issue with parsing the file. That file is 19,000 plus lines with a large amount of deeply nested code blocks.

Does the production build have sourcemaps enabled?

Also of note, is that the library in question appears to be a NodeJS platform library and not a browser platform library. While this library can be made to work on a browser, there is a difference between working and actually targeting a particular platform.

@kzc
Copy link

kzc commented Nov 18, 2018

@clydin @filipesilva In the interest of making it easier to debug future minifier issues could this project add a new --no-minify flag to the CLI that could work in conjunction with --prod or development mode to produce unminified code? If this process is correct it would produce identical webpack output as would have been the input to Terser.

Also, consider setting the Terser ecma options for parse, compress and output separately. No harm in making parse use the most permissive ecma version, while limiting compress and output. (I mistakenly referred to a Terser mangle ecma option above - presently there is no such option).

@RyanHow
Copy link
Author

RyanHow commented Nov 19, 2018

That appears to be a typescript issue with parsing the file. That file is 19,000 plus lines with a large amount of deeply nested code blocks.

Yes, it is a huge file. It is generated by PEG.js.

Does the production build have sourcemaps enabled?

Nope

Also of note, is that the library in question appears to be a NodeJS platform library and not a browser platform library. While this library can be made to work on a browser, there is a difference between working and actually targeting a particular platform.

As far as I know, it is designed to work in the browser and node. I can't say libraries are my speciality, how does a single library go about targeting node and a browser? rather than just being made to work in both?

The issue in question though (The build optimizer hanging) is only occurring on a single file. And that file doesn't contain any imports. So I don't think this one is a platform issue.

I think you are correct on the typescript parsing. That seems to be where the issue is happening. So I guess the next step is to follow up with the typescript team?

@filipesilva
Copy link
Contributor

filipesilva commented Nov 20, 2018

@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 #12153 and microsoft/TypeScript#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.

@filipesilva filipesilva added the needs: discussion On the agenda for team meeting to determine next steps label Nov 20, 2018
@RyanHow
Copy link
Author

RyanHow commented Nov 20, 2018

Thanks so much for the support and investigation despite the custom webpack config.

I've logged an issue here too and we'll see how it goes.

microsoft/TypeScript#28617

Would there be any simple workarounds in the interim apart from disabling the build optimizer?

Regarding the library ECMA version, they've fixed the library so it is valid ES5, but we might not all be so lucky. It's easy to debug once you know how, but if it is possible for the CLI to give more informative output that would speed the process up a lot!

Just a note: Apparently there were quite a few other ES6 issues in the library, but Terser was only complaining about that one. So if the idea was to run it through a linter or something else to pick up those issues, it may end up picking up more than it needs to?

So it would need to use the Terser error to get the line number, but without source maps I'm not sure how you would reference it back to the source file?

Anyway... I love the idea, and thanks so much for the support.

@kzc
Copy link

kzc commented Nov 20, 2018

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.

@hansl hansl removed the needs: discussion On the agenda for team meeting to determine next steps label Nov 29, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jan 10, 2019
@filipesilva filipesilva changed the title Build hangs indefinitely on 3rd party library and Terser fails with --buildOptimizer=false Third party ES2015 library causes build to hang/fail Mar 7, 2019
@filipesilva filipesilva changed the title Third party ES2015 library causes build to hang/fail Third party ES2015 library causes build to hang/fail/be broken Mar 7, 2019
@filipesilva filipesilva pinned this issue Mar 7, 2019
@filipesilva filipesilva unpinned this issue Mar 7, 2019
@moonthedev
Copy link

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.

Thank you very much! I could find the problem in my code with your suggestion

@thematho
Copy link

Hi
I'm having the same issue but in my case the library with es2017 code is a third party library, so I can't modify the code, and also I shouldn't due is a valid sintax just need support for es2017.

angular-cli is not working with the default terser options check here and here
Error:

ERROR in vendor-es2015.262ed28c0499c356a1a5.js from Terser
Unexpected token: punc ()) [./node_modules/@angular-devkit/build-optimizer/src/build-optimizer/webpack-loader.js??ref--8-0!./node_modules/xxxxxxxxxxxx.js:19,0][vendor-es2015.262ed28c0499c356a1a5.js:46989,0]

Third party library code example:

export class MyField extends MyMixin(
 ControlMixin(VMixin(CMixin(SMixin(OMixin(MyElement))))), // <- when removing this trailing comma the error disappear
) {
//....

This is a set of web-components I'm trying to consume from angular, all of them using trailing commas in function parameters.
There is any way to override this terser option from angular.json or throw command line?

Reference issue Unexpected token: punc ()) with comma after last param in func calls #412

@martin31821
Copy link

@thematho I'm facing the same issue with the postprocessing library after upgrading to angular 8.
Did you find a solution?

@thematho
Copy link

thematho commented Sep 7, 2019

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

@tonywang160
Copy link

I also have the same issue with
node --version
v10.11.0

npm --version
6.4.1

Angular CLI: 8.3.4
Node: 10.11.0
OS: win32 x64
Angular: 8.2.6
animations, common, compiler, core, forms, platform-browser, platform-browser-dynamic, router
@angular/flex-layout": "^8.0.0-beta.27",
Angular Material: 8.1.4
material, cdk
Angular flex-layout: 8.0.0-beta.27

devDependencies": {
"@angular-devkit/build-angular": "~0.803.4",
"@angular/cli": "^8.3.4",
"@angular/compiler-cli": "~8.2.6",
"@angular/language-service": "~8.2.6",
"@types/node": "~8.9.4",
"ts-node": "~7.0.0",
"tslint": "~5.11.0",
"typescript": "~3.5.3"

I was able to compile with ng build --prod --buildOptimizer=false --optimization=false
but would like to turn optimizer on.

Did you find a solution?

@martin31821
Copy link

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

@tonywang160
Copy link

updating the dependency caused the problem

Could you let me know which dependency did you updated? My package.json file is below and I think all the packages are updated.

{
"name": "yd-web",
"version": "1.0.0",
"scripts": {
"ng": "ng",
"start": "ng serve",
"build": "ng build",
"test": "ng test",
"lint": "ng lint",
"e2e": "ng e2e"
},
"private": true,
"dependencies": {
"@angular/animations": "^8.2.6",
"@angular/cdk": "^8.1.4",
"@angular/common": "~8.2.6",
"@angular/compiler": "~8.2.6",
"@angular/core": "~8.2.6",
"@angular/flex-layout": "^8.0.0-beta.27",
"@angular/forms": "~8.2.6",
"@angular/material": "^8.1.4",
"@angular/platform-browser": "~8.2.6",
"@angular/platform-browser-dynamic": "~8.2.6",
"@angular/router": "~8.2.6",
"hammerjs": "^2.0.8",
"ng-material-multilevel-menu": "^4.10.8",
"ngx-markdown": "^8.1.0",
"reframe.js": "^2.2.5",
"rxjs": "~6.5.3",
"rxjs-observable-store": "^2.0.1",
"tslib": "^1.9.0",
"zone.js": "~0.9.1"
},
"devDependencies": {
"@angular-devkit/build-angular": "~0.803.4",
"@angular/cli": "^8.3.4",
"@angular/compiler-cli": "~8.2.6",
"@angular/language-service": "~8.2.6",
"@types/jasmine": "~2.8.8",
"@types/jasminewd2": "~2.0.3",
"@types/node": "~8.9.4",
"codelyzer": "^5.0.1",
"jasmine-core": "~2.99.1",
"jasmine-spec-reporter": "~4.2.1",
"karma": "~4.0.0",
"karma-chrome-launcher": "~2.2.0",
"karma-coverage-istanbul-reporter": "~2.0.1",
"karma-jasmine": "~1.1.2",
"karma-jasmine-html-reporter": "^0.2.2",
"protractor": "~5.4.0",
"ts-node": "~7.0.0",
"tslint": "~5.11.0",
"typescript": "~3.5.3"
}
}

@rochapablo

This comment has been minimized.

@thematho

This comment has been minimized.

@BorntraegerMarc
Copy link

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

@kyliau kyliau added triage #1 needs: discussion On the agenda for team meeting to determine next steps labels May 28, 2020
@alan-agius4 alan-agius4 removed the needs: discussion On the agenda for team meeting to determine next steps label Jun 18, 2020
@alan-agius4
Copy link
Collaborator

Thanks for reporting this issue. This issue is now obsolete due to changes in the recent releases. Please update to the most recent Angular CLI version.

If the problem persists after upgrading, please open a new issue, provide a simple repository reproducing the problem, and describe the difference between the expected and current behavior.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.