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.

Sign up for GitHub

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

Jump to bottom

Build: compile deps to ES5 when generating browser file (fixes #11504) #11505

Merged
merged 2 commits into from Mar 15, 2019
Merged

Build: compile deps to ES5 when generating browser file (fixes #11504) #11505

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3bf2e7
Build: compile deps to ES5 when generating browser file (fixes #11504)
not-an-aardvark Mar 13, 2019
6685a48
Don't compile lodash
not-an-aardvark Mar 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 16 additions & 3 deletions Makefile.js
View file
Open in desktop
Expand Up @@ -25,7 +25,8 @@ const lodash = require("lodash"),
semver = require("semver"),
ejs = require("ejs"),
loadPerf = require("load-perf"),
yaml = require("js-yaml");
yaml = require("js-yaml"),
CLIEngine = require("./lib/cli-engine");

const { cat, cd, cp, echo, exec, exit, find, ls, mkdir, pwd, rm, test } = require("shelljs");

Expand Down Expand Up @@ -440,8 +441,7 @@ function lintMarkdown(files) {
* @returns {Object} Output from each formatter
*/
function getFormatterResults() {
const CLIEngine = require("./lib/cli-engine"),
stripAnsi = require("strip-ansi");
const stripAnsi = require("strip-ansi");

const formatterFiles = fs.readdirSync("./lib/formatters/"),
cli = new CLIEngine({
Expand Down Expand Up @@ -580,6 +580,19 @@ target.test = function() {

target.webpack();

const browserFileLintOutput = new CLIEngine({
Copy link
Member

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.

Copy link
Member Author

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.

useEslintrc: false,
ignore: false,
allowInlineConfig: false,
baseConfig: { parserOptions: { ecmaVersion: 5 } }
}).executeOnFiles([`${BUILD_DIR}/eslint.js`]);

if (browserFileLintOutput.errorCount > 0) {
echo(`error: Failed to lint ${BUILD_DIR}/eslint.js as ES5 code`);
echo(CLIEngine.getFormatter("stylish")(browserFileLintOutput.results));
errors++;
}

lastReturn = exec(`${getBinFile("karma")} start karma.conf.js`);
if (lastReturn.code !== 0) {
errors++;
Expand Down
5 changes: 2 additions & 3 deletions webpack.config.js
View file
Open in desktop
Expand Up @@ -15,12 +15,11 @@ module.exports = {
module: {
rules: [
{
test: /\.js$/u,
test: /\.m?js$/u,
loader: "babel-loader",
options: {
presets: ["@babel/preset-env"]
},
exclude: /node_modules/u
}
}
]
},
Expand Down