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
Build: compile deps to ES5 when generating browser file (fixes #11504) #11505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but can we ignore some large-size libraries like "lodash"? These libraries are already in ES5 and compiling them will slow down the build speed.
I updated the PR to ignore |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general, just left one comment. Thanks!
@@ -580,6 +580,19 @@ target.test = function() { | |||
|
|||
target.webpack(); | |||
|
|||
const browserFileLintOutput = new CLIEngine({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly about this, but would it be simpler/less time-consuming to just pull in espree here and do a parse? I don't understand what checks might be needed in the linting when we're not using any configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. I had been thinking running the linter would be useful if there were other ES5-only checks we wanted to enable in addition to checking for ES5 syntax, but since no such checks are actually enabled, using espree
on its own would probably be simpler.
As a separate PR, I wonder if it would be worthwhile to have a special-case optimization in Linter
that skips traversing the AST when there are no rules enabled. That seems like it would make the performance of "ESLint with no rules enabled" similar to the performance of espree.
What is the purpose of this pull request? (put an "X" next to item)
[x] Bug fix (#11504)
What changes did you make? (Give an overview)
This updates the webpack config to compile dependencies to ES5, to ensure that the final output file is runnable in ES5 browsers. It tests this behavior by running ESLint on the output file.
Is there anything you'd like reviewers to focus on?
Nothing in particular