Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

feat!: enforce use of strict (===) equality #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Apr 8, 2020

ESLint can enforce this for us with the built-in "eqeqeq" rule, which you might not know about if you'd search for "strict", "equality" or "equal" in the rules list.

Idea is to avoid coercion due to ==/!= which can be a source of bugs, except in the case of null, where x == null is a convenient and idiomatic shorthand for x === null || x === undefined (you can still do an explicit x === null check if that's what you want).

Docs: https://eslint.org/docs/rules/eqeqeq

Closes: https://github.com/liferay/eslint-config-liferay/issues/158

ESLint can enforce this for us with the built-in "eqeqeq" rule, which
you might not know about if you'd search for "strict", "equality" or
"equal" in the rules list.

Idea is to avoid coercion due to `==`/`!=` which can be a source of
bugs, except in the case of `null`, where `x == null` is a convenient
and idiomatic shorthand for `x === null || x === undefined` (you can
still do an explicit `x === null` check if that's what you want).

Docs: https://eslint.org/docs/rules/eqeqeq

Closes: https://github.com/liferay/eslint-config-liferay/issues/158
@wincent wincent added the feature label Apr 8, 2020
@wincent
Copy link
Contributor Author

wincent commented Apr 8, 2020

FWIW, tested this in liferay-portal with:

cp ~/code/eslint-config-liferay/index.js ~modules/node_modules/eslint-config-liferay
portool yarn run liferay-npm-scripts lint  

Finds a bunch of errors:

✖ 364 problems (364 errors, 0 warnings)
  21 errors and 0 warnings potentially fixable with the `--fix` option.

Autofix diff is (14 files changed, 24 insertions(+), 21 deletions(-)):

https://gist.github.com/wincent/9b1925268aa56312106908b892c09138

Not-autofixable stuff is like this:

		return pauseNodes.filter(({id}) =>
			nodeKeys.find(node => node.id == id)
		);

All of these require human review to be "sure" they're safe, and even then I don't think we can be sure. I think we just have to ship it, run the tests, and watch for regressions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enforce use of "strict equality"
1 participant