Speed up ESLint + Prettier

[pdehaan]

The majestic Mr @gregglind was commenting that the prettier and eslint --fix were a bit slow and was wondering if we could somehow optimize...

Filing this issue as a conversation starter and dumping ground for research findings and possible solutions.

[pdehaan]

Here's some initial perf numbers and debug info from $ npm run eslint:

$ time DEBUG=eslint:cli-engine npm run eslint

> shield-studies-addon-utils@5.0.0-alpha1 eslint /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils
> eslint . --ext js --ext json

  eslint:cli-engine Linting ./.eslintrc.js +3ms
  eslint:cli-engine Linting ./bin/.eslintrc.js +0ms
  eslint:cli-engine Linting ./bin/copyStudyUtils.js +0ms
  eslint:cli-engine Linting ./bin/documentSchema.js +1ms
  eslint:cli-engine Linting ./bin/schemaToInterface.js +1ms
  eslint:cli-engine Linting ./bin/verifyWeeSchema.js +0ms
  eslint:cli-engine Linting ./bin/wee-schema-schema.json +5ms
  eslint:cli-engine Linting ./examples/small-study/src/.eslintrc.js +2ms
  eslint:cli-engine Linting ./examples/small-study/src/background.js +8ms
  eslint:cli-engine Linting ./examples/small-study/src/manifest.json +10ms
  eslint:cli-engine Linting ./examples/small-study/src/studySetup.js +3ms
  eslint:cli-engine Linting ./examples/small-study/web-ext-config.js +1ms
  eslint:cli-engine Linting ./test-addon/src/.eslintrc.js +5ms
  eslint:cli-engine Linting ./test-addon/src/background.js +3ms
  eslint:cli-engine Linting ./test-addon/src/extension-page-for-tests/page.js +1ms
  eslint:cli-engine Linting ./test-addon/src/manifest.json +1ms
  eslint:cli-engine Linting ./test-addon/src/studySetup.js +0ms
  eslint:cli-engine Linting ./test-addon/web-ext-config.js +0ms
  eslint:cli-engine Linting ./test/functional/browser.study.api.js +1ms
  eslint:cli-engine Linting ./test/functional/test-addon.js +0ms
  eslint:cli-engine Linting ./test/functional/utils.js +0ms
  eslint:cli-engine Linting ./testUtils/.eslintrc.js +0ms
  eslint:cli-engine Linting ./testUtils/executeJs.js +1ms
  eslint:cli-engine Linting ./testUtils/nav.js +0ms
  eslint:cli-engine Linting ./testUtils/setupWebdriver.js +0ms
  eslint:cli-engine Linting ./testUtils/telemetry.js +0ms
  eslint:cli-engine Linting ./testUtils/ui.js +1ms
  eslint:cli-engine Linting ./testUtils/wip.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/prefs/api.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/prefs/schema.json +0ms
  eslint:cli-engine Linting ./webExtensionApis/study/fakeApi.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/schema.json +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/src/index.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/src/jsonschema.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/src/sampling.js +0ms
  eslint:cli-engine Linting ./webExtensionApis/study/src/studyUtils.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/src/studyUtilsBootstrap.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/src/telemetry.js +1ms
  eslint:cli-engine Linting ./webExtensionApis/study/webpack.config.js +0ms
  eslint:cli-engine Linting complete in: 1547ms +6ms

...

DEBUG=eslint:cli-engine npm run eslint  2.58s user 0.42s system 79% cpu 3.757 total

$ git rev-parse --short HEAD # 9208966 ("develop" branch)

So it looks like simply running $ npm run eslint takes somewhere between 1.547 and 3.757 seconds. Nothing immediately stands out as being hugely offensive (performance wise). Most files are linted in 1-3ms, with a few outliers at 5ms, 8ms, and 10ms; which all seem reasonable.

Not sure if there is anything else we may want to add to the .eslintignore file:

$ cat .eslintignore

# do not lint/format generated artifacts
dist/
webExtensionApis/study/api.js
package-lock.json
# do not lint/format bundled util libraries
examples/test-addon/src/privileged/prefs/
examples/test-addon/src/privileged/study/
# makes sure that eslintrc.js gets linted/formatted
!.eslintrc.js
# don't lint/format code the directory where get-firefox stores the Firefox binaries
/firefox
# don't lint/format package.json since npm install formats it differently by default
package.json
# don't lint node_modules
node_modules

# re #169, skip the misc dir
misc/

[gregglind]

Eslint has a timing debugger as well. It was whitespace fix that took all the time.

In my branch I added a SKIPLINT flag for speeding up certain kinds of runs, will link tomorrow.

On Wed, May 9, 2018 at 6:47 PM Peter deHaan notifications@github.com wrote:

Here's some initial perf numbers and debug info from $ npm run eslint:

$ time DEBUG=eslint:cli-engine npm run eslint

shield-studies-addon-utils@5.0.0-alpha1 eslint /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils> eslint . --ext js --ext json

eslint:cli-engine Linting ./.eslintrc.js +3ms eslint:cli-engine Linting ./bin/.eslintrc.js +0ms eslint:cli-engine Linting ./bin/copyStudyUtils.js +0ms eslint:cli-engine Linting ./bin/documentSchema.js +1ms eslint:cli-engine Linting ./bin/schemaToInterface.js +1ms eslint:cli-engine Linting ./bin/verifyWeeSchema.js +0ms eslint:cli-engine Linting ./bin/wee-schema-schema.json +5ms eslint:cli-engine Linting ./examples/small-study/src/.eslintrc.js +2ms eslint:cli-engine Linting ./examples/small-study/src/background.js +8ms eslint:cli-engine Linting ./examples/small-study/src/manifest.json +10ms eslint:cli-engine Linting ./examples/small-study/src/studySetup.js +3ms eslint:cli-engine Linting ./examples/small-study/web-ext-config.js +1ms eslint:cli-engine Linting ./test-addon/src/.eslintrc.js +5ms eslint:cli-engine Linting ./test-addon/src/background.js +3ms eslint:cli-engine Linting ./test-addon/src/extension-page-for-tests/page.js +1ms eslint:cli-engine Linting ./test-addon/src/manifest.json +1ms eslint:cli-engine Linting ./test-addon/src/studySetup.js +0ms eslint:cli-engine Linting ./test-addon/web-ext-config.js +0ms eslint:cli-engine Linting ./test/functional/browser.study.api.js +1ms eslint:cli-engine Linting ./test/functional/test-addon.js +0ms eslint:cli-engine Linting ./test/functional/utils.js +0ms eslint:cli-engine Linting ./testUtils/.eslintrc.js +0ms eslint:cli-engine Linting ./testUtils/executeJs.js +1ms eslint:cli-engine Linting ./testUtils/nav.js +0ms eslint:cli-engine Linting ./testUtils/setupWebdriver.js +0ms eslint:cli-engine Linting ./testUtils/telemetry.js +0ms eslint:cli-engine Linting ./testUtils/ui.js +1ms eslint:cli-engine Linting ./testUtils/wip.js +1ms eslint:cli-engine Linting ./webExtensionApis/prefs/api.js +1ms eslint:cli-engine Linting ./webExtensionApis/prefs/schema.json +0ms eslint:cli-engine Linting ./webExtensionApis/study/fakeApi.js +1ms eslint:cli-engine Linting ./webExtensionApis/study/schema.json +1ms eslint:cli-engine Linting ./webExtensionApis/study/src/index.js +1ms eslint:cli-engine Linting ./webExtensionApis/study/src/jsonschema.js +1ms eslint:cli-engine Linting ./webExtensionApis/study/src/sampling.js +0ms eslint:cli-engine Linting ./webExtensionApis/study/src/studyUtils.js +1ms eslint:cli-engine Linting ./webExtensionApis/study/src/studyUtilsBootstrap.js +1ms eslint:cli-engine Linting ./webExtensionApis/study/src/telemetry.js +1ms eslint:cli-engine Linting ./webExtensionApis/study/webpack.config.js +0ms eslint:cli-engine Linting complete in: 1547ms +6ms

...

DEBUG=eslint:cli-engine npm run eslint 2.58s user 0.42s system 79% cpu 3.757 total

$ git rev-parse --short HEAD # 9208966 ("develop" branch)

So it looks like simply running $ npm run eslint takes somewhere between 1.547 and 3.757 seconds. Nothing immediately stands out as being hugely offensive (performance wise). Most files are linted in 1-3ms, with a few outliers at 5ms, 8ms, and 10ms; which all seem reasonable.

Not sure if there is anything else we may want to add to the .eslintignore file:

$ cat .eslintignore

do not lint/format generated artifacts

dist/ webExtensionApis/study/api.js package-lock.json# do not lint/format bundled util libraries examples/test-addon/src/privileged/prefs/ examples/test-addon/src/privileged/study/# makes sure that eslintrc.js gets linted/formatted!.eslintrc.js# don't lint/format code the directory where get-firefox stores the Firefox binaries /firefox# don't lint/format package.json since npm install formats it differently by default package.json# don't lint node_modules node_modules

re #169, skip the misc dir

misc/

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mozilla/shield-studies-addon-utils/issues/175#issuecomment-387908195, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKAj6VqLwqfqZSXkulgOXMlWiSLNVtkks5tw4APgaJpZM4T5JsB .

[pdehaan]

Running $ npm run format, takes around 6-7 seconds.

✗ time npm run format

> shield-studies-addon-utils@5.0.0-alpha1 format /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils
> prettier '**/*.{css,js,jsm,json,md}' --trailing-comma=all --ignore-path=.eslintignore --write

.eslintrc.js 68ms
bin/.eslintrc.js 4ms
bin/copyStudyUtils.js 30ms
bin/documentSchema.js 42ms
bin/schemaToInterface.js 22ms
bin/verifyWeeSchema.js 35ms
bin/wee-schema-schema.json 20ms
docs/api.md 201ms
docs/development-on-the-utils.md 14ms
docs/engineering.md 211ms
docs/survival.md 57ms
docs/tutorial.md 22ms
docs2.md 134ms
examples/small-study/README.md 13ms
examples/small-study/src/.eslintrc.js 4ms
examples/small-study/src/background.js 18ms
examples/small-study/src/manifest.json 13ms
examples/small-study/src/studySetup.js 24ms
examples/small-study/web-ext-config.js 5ms
README.md 65ms
test-addon/src/.eslintrc.js 3ms
test-addon/src/background.js 23ms
test-addon/src/extension-page-for-tests/page.js 3ms
test-addon/src/manifest.json 6ms
test-addon/src/studySetup.js 20ms
test-addon/web-ext-config.js 4ms
test/functional/browser.study.api.js 80ms
test/functional/test-addon.js 21ms
test/functional/utils.js 6ms
testUtils/.eslintrc.js 2ms
testUtils/executeJs.js 12ms
testUtils/nav.js 4ms
testUtils/setupWebdriver.js 19ms
testUtils/telemetry.js 18ms
testUtils/ui.js 36ms
testUtils/wip.js 10ms
webExtensionApis/prefs/api.js 4ms
webExtensionApis/prefs/schema.json 8ms
webExtensionApis/study/fakeApi.js 36ms
webExtensionApis/study/schema.json 44ms
webExtensionApis/study/src/index.js 60ms
webExtensionApis/study/src/jsonschema.js 11ms
webExtensionApis/study/src/sampling.js 17ms
webExtensionApis/study/src/studyUtils.js 74ms
webExtensionApis/study/src/studyUtilsBootstrap.js 25ms
webExtensionApis/study/src/telemetry.js 18ms
webExtensionApis/study/webpack.config.js 5ms

> shield-studies-addon-utils@5.0.0-alpha1 postformat /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils
> run-p lint:fixpack eslint-fix

> shield-studies-addon-utils@5.0.0-alpha1 eslint-fix /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils
> npm run eslint -- --fix

> shield-studies-addon-utils@5.0.0-alpha1 lint:fixpack /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils
> fixpack  # cleans up package.json

package.json already clean!

> shield-studies-addon-utils@5.0.0-alpha1 eslint /Users/pdehaan/dev/github/mozilla/shield-studies-addon-utils
> eslint . --ext js --ext json "--fix"

...

✖ 21 problems (0 errors, 21 warnings)

npm run format  6.54s user 1.00s system 102% cpu 7.334 total

It's a bit hard to tell, but if I re-run npm run format over and over, we seem to have a small handful of files that will get modified by prettier and then rewritten by eslint --fix:

1. bin/schemaToInterface.js
2. test/functional/browser.study.api.js
3. test/functional/test-addon.js
4. testUtils/executeJs.js
5. testUtils/nav.js
6. testUtils/setupWebdriver.js
7. testUtils/telemetry.js
8. testUtils/ui.js

I'd probably remove fixpack, but doubt that it'd speed it up considerably.

[pdehaan]

Another option we could look at is something like husky + lint-staged and only lint the modified files which would speed things up exponentially (and automatically format+lint files on precommit or prepush git hooks).

---

UPDATE: I'm starting to wonder how great an idea this is. 💡

We would only run npm run format on a precommit or prepush git hook, which would probably speed it up greatly. Similarly, using lint-staged we only run fixpack whenever the package.json file has changed, since otherwise it would be an expensive noop.

Another possible optimization would be replacing calls w/ the expensive npm run build with npm run fast-build, which does the same thing, but without the prebuild and postbuild lifecycle hooks:

    "prepare": "fixpack && npm run build",
    "presmall-study:run": "npm run build && npm run small-study:bundle-utils",
    "pretest": "npm run build && npm run test-addon:bundle-utils && npm run test-addon:build",

Where the prebuild and postbuild currently look like:

    "postbuild": "if [ -z ${SKIPLINT} ]; then npm run format; fi",
    "postformat": "run-p lint:fixpack eslint-fix",
    "prebuild": "if [ -z ${SKIPLINT} ]; then npm run lint; fi",

So, the current flow of running something like $ npm test would do something like:

1. $ npm test:
   1. runs pretest script which calls npm run build.
      1. runs prebuild script which calls npm run lint (unless passed a SKIPLINT env var).
         1. npm run lint runs ESLint and fixpack.
      2. runs build script which does stuff.
      3. runs postbuild which calls npm run format (unless passed a SKIPLINT env var).
         1. npm run format calls prettier to reformat entire code base (.css, .js, .jsm, .json, and *.md files).
         2. runs postformat which runs fixpack (again) and ESLint (again) with --fix flag to reformat code base (again).
   2. runs test script which calls mocha and runs the actual tests.

[pdehaan]

I ran npm run format (without any other npm lifecycle hooks), and it looks like the only diffs were stuff like the following:

--- a/testUtils/nav.js
+++ b/testUtils/nav.js
@@ -4,7 +4,7 @@ const firefox = require("selenium-webdriver/firefox");
 const Context = firefox.Context;

 module.exports.nav = {
-  gotoURL: async(driver, url) => {
+  gotoURL: async (driver, url) => {

[pdehaan]

Another option is something like prettier-eslint, which:

Formats your JavaScript using prettier followed by eslint --fix

I haven't tried it before, so can't comment on efficiency or how the configs work.