search

wikimedia / eslint-plugin-no-jquery

Control allowance of certain jQuery functions, and suggest or autofix alternatives.
MIT License
32 stars 8 forks source link

'no-ready-shorthand' and 'no-on-ready' have a small disagreement #244

Closed drorsnir closed 4 years ago

drorsnir commented 4 years ago

no-ready-shorthand.md says that the following is OK: $( document ).on( 'ready', function () {} );

However, no-on-ready says it's not.

edg2s commented 4 years ago

I don't think it's a problem in general for something that is valid in one rule to be invalid in another rule. Rules only lint against invalid patterns, the valid patterns are just suggestions. Even in the case of rules that provide fixes, it is ok for the fixed output to be invalid and then fixed by another rule: ESLint automatically runs multiple fix passes until there is no change.

To give a generic example you could have a rule no-foo that replaces foo with bar, but also have a rule no-bar that replaces bar with quux. If foo was deprecated in version 2 and bar was deprecated in version 3, then you would enable the no-foo for upgrading from version 2 and both rules for upgrading from version 3. With both rules enabled, foo would be replaced with quux.

Do you have a specific example of where this is causing problems?

drorsnir commented 4 years ago

Exactly as I said - I think $( document ).on( 'ready', function () {} ); should just be removed from no-ready-shorthand as a valid option. This is not really a big deal, as there's no real need to use either.

edg2s commented 4 years ago

$( document ).on( 'ready', function () {} ); is perfectly valid code if the only rule you have enabled is no-ready-shorthand. Rules are supposed to be independent of each other, as described above. Note that valid code is not he same as recommended code, it is just illustrating what patterns will not trigger errors.

This may be a case where we should link from one rule to another with a "See also" section, or have a longer description about how the rules are related, but the as the valid example is valid I'll close this task for now.

Feel free to re-open if you think I've missed something.

Thanks,

drorsnir commented 4 years ago

OK, I get it now. Thanks for the thorough explanation!