Skip to content

Add ES lint support to identify poorly written Promises

kushalpandya requested to merge 27486-eslint-promises into master

What does this MR do?

Enables ESLint to check if Promises are written correctly using eslint-plugin-promise.

Are there points in the code the reviewer needs to double check?

Merging this MR will cause lint:javascript job to fail with 15 errors, we may need another MR that fixes those offences first before we can merge this MR. Linter offences are fixed with !9991 (merged).

Also, two points worth noting for this MR;

  • This MR only enables catch-or-return rule of the linter which identifies missing .catch() from all Promises.
  • This Linter plugin doesn't support jQuery Deferreds (where we have .fail() instead of .catch()), so it requires to be disabled for such cases.

Why was this MR needed?

Currently we have no static analysis in place to identify poorly written Promises (examples for the same can be found here), while our Promise polyfill will complain for Promises without catch() callback, it can be identified only when karma tests are run, and those complaints do not lead to Karma test failure, though. Using ESLint to identify such problems ensures we never have unhandled Promise errors into production code. 🙂

Screenshots

ESLint_Promise

image

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Related EE MR https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1682

#27486 (moved)

Merge request reports