search

bertramdev / asset-pipeline

The core implementation of the asset pipeline for the jvm
193 stars 91 forks source link

ES6 detection false-positives and no way to manually disable Babel #260

Closed jwueller closed 4 years ago

jwueller commented 4 years ago

This line currently kills performance of our application in development:

https://github.com/bertramdev/asset-pipeline/blob/46e82403955168a2c667c8712c18c381701121e4/asset-pipeline-core/src/main/groovy/asset/pipeline/processors/BabelJsProcessor.groovy#L82

We do not use ES6 anything, because TypeScript already compiles it down to ES5 for us. But there are code samples in some library comments (e.g. React), that incorrectly get detected as ES6. Optimally, this would properly honor JavaScript syntax to see if the keywords are actually present in the source, but even outside of that issue, it would be nice to be able to explicitly disable the whole thing, because we don't need any pre-processing and would like to not take the extra performance hit when we know it doesn't do anything for us.

Babel inside Nashorn, for some reason, can take ~15 minutes to execute with our codebase, where the TypeScript compiler finishes within a few seconds. Right now, our only real choice is pre-processing the source to remove comments, so that we don't get false-positives, which is also unnecessarily slow.

davydotcom commented 4 years ago

so here was a bug with the enableES6 config option not letting you disable babel. you can explicitly set this to false to stop babel.

davydotcom commented 4 years ago

in 3.2.3

jwueller commented 4 years ago

Sadly, version 3.2.3 does not seem to fix this issue and I would like to re-open this.

The enableES6 config option will only be checked if no ES6 keywords (which in this case are a false-positive anyway) are detected: https://github.com/bertramdev/asset-pipeline/blob/3a68b48d6e5fe3fd46503e42be3ceba073805cfe/asset-pipeline-core/src/main/groovy/asset/pipeline/processors/BabelJsProcessor.groovy#L85

Therefore, there is no way to explicitly disable Babel preprocessing that I can see. As a result, Nashorn is still being invoked, causing a massively slow parse of the Babel transpiler, that isn't even being used afterwards.

As an aside, Nashorn is also going to be removed from the JDK, causing asset-pipeline to print this warning:

Warning: Nashorn engine is planned to be removed from a future JDK release

So maybe it should not be used in any case to convert to ES6, even if Babel is enabled?

jwueller commented 4 years ago

Thank you very much for the speedy merge and release! Version 3.2.4 is tested working!

davydotcom commented 4 years ago

No problem, thanks for the correction. Apologies for the incorrect first attempt as I was juggling cleanup elsewhere too.