Mailing List Archive

[Moving to wikitech-l; mediawiki-l is a little off-topic.]

On Mon, 26 Jul 2021 at 21:27, Yaron Koren <yaron57@gmail.com> wrote:

> Hi,
>
> I've been trying to get rid of the ESLint warnings for the JavaScript code
> in some of my extensions, when they go through Jenkins validation. One
> warning that appears fairly often is this one:
>
> Where possible, maintain application state in JS to avoid slower DOM
> queries
> no-jquery/no-class-state
>
>
> I'm not sure if this is a warning that's specific to Wikimedia code, but
> doing a web search on it brings up this Wikimedia help page as the only
> real result:
>
>
> https://github.com/wikimedia/eslint-plugin-no-jquery/blob/master/docs/rules/no-class-state.md
>

Yes, we forked the dead "jquery" upstream eslint plugin and have expanded
it significantly. In general, the plugin's purpose
<https://www.npmjs.com/package/eslint-plugin-no-jquery> is to discourage
use of jQuery functions, especially where the functions are deprecated or
have faster native equivalents. (Almost all uses of jQuery are no longer
necessary given that the vast majority of the Web only runs on
modern-enough browsers.)


This page is rather confusing. It says that the warning comes when calling
> either hasClass() or toggleClass() on a jQuery element. That part makes
> sense, but then the suggested alternatives are strange. The page says that
> the following are some examples of bad code:
>
> $( 'div' ).hasClass();
> $div.hasClass();
>
> In their place, it suggests the following:
>
> hasClass();
> [].hasClass();
> div.hasClass();
>

No, it doesn't. It says that our code is clever enough to not think that
these false positives are issues that you need to fix. It's not saying you
should use a method called hasClass(); it's saying that you should maintain
state inside JS; this is principally a performance/code smell test.

If your code is triggered from a non-JS DOM (e.g. painted from PHP), the
first time you grab state from the DOM is unavoidable (and so you'd use an
inline disable), but thereafter you should keep track of such details in
JS. An example of this is in the initialisation code for Notifications
("Echo"), where it has to grab the state from the DOM
<https://gerrit.wikimedia.org/g/mediawiki/extensions/Echo/+/HEAD/modules/ext.echo.init.js#46>
for a couple of items, but does so only once.

Sorry that this is confusing! We could put together a narrow JS tips and
tricks page and link to that from the linter, but most of these have been
fixed over the years since we introduced this in Wikimedia production code
so there's not been much call.

J.
--
*James D. Forrester* (he/him <http://pronoun.is/he> or they/themself
<http://pronoun.is/they/.../themself>)
Wikimedia Foundation <https://wikimediafoundation.org/>

Hi James,

Thank you, that is helpful. I'll look through my code; my guess is that the
best solution (at least for now) is to add that "eslint-disable" call to
many or even most of my hasClass() calls.

By the way, I think the current wording in the documentation is misleading:
the page I linked to lists the two kinds of function calls as "incorrect"
and "correct" - which I think implies something stronger than "this code
will lead to a validation warning" and "this code won't". (Most of the
so-called "correct" code won't work at all.)

-Yaron

On Sat, Aug 14, 2021 at 8:34 AM James Forrester <jforrester@wikimedia.org>
wrote:

> [Moving to wikitech-l; mediawiki-l is a little off-topic.]
>
> On Mon, 26 Jul 2021 at 21:27, Yaron Koren <yaron57@gmail.com> wrote:
>
>> Hi,
>>
>> I've been trying to get rid of the ESLint warnings for the JavaScript
>> code in some of my extensions, when they go through Jenkins validation. One
>> warning that appears fairly often is this one:
>>
>> Where possible, maintain application state in JS to avoid slower DOM
>> queries
>> no-jquery/no-class-state
>>
>>
>> I'm not sure if this is a warning that's specific to Wikimedia code, but
>> doing a web search on it brings up this Wikimedia help page as the only
>> real result:
>>
>>
>> https://github.com/wikimedia/eslint-plugin-no-jquery/blob/master/docs/rules/no-class-state.md
>>
>
> Yes, we forked the dead "jquery" upstream eslint plugin and have expanded
> it significantly. In general, the plugin's purpose
> <https://www.npmjs.com/package/eslint-plugin-no-jquery> is to discourage
> use of jQuery functions, especially where the functions are deprecated or
> have faster native equivalents. (Almost all uses of jQuery are no longer
> necessary given that the vast majority of the Web only runs on
> modern-enough browsers.)
>
>
> This page is rather confusing. It says that the warning comes when calling
>> either hasClass() or toggleClass() on a jQuery element. That part makes
>> sense, but then the suggested alternatives are strange. The page says that
>> the following are some examples of bad code:
>>
>> $( 'div' ).hasClass();
>> $div.hasClass();
>>
>> In their place, it suggests the following:
>>
>> hasClass();
>> [].hasClass();
>> div.hasClass();
>>
>
> No, it doesn't. It says that our code is clever enough to not think that
> these false positives are issues that you need to fix. It's not saying you
> should use a method called hasClass(); it's saying that you should maintain
> state inside JS; this is principally a performance/code smell test.
>
> If your code is triggered from a non-JS DOM (e.g. painted from PHP), the
> first time you grab state from the DOM is unavoidable (and so you'd use an
> inline disable), but thereafter you should keep track of such details in
> JS. An example of this is in the initialisation code for Notifications
> ("Echo"), where it has to grab the state from the DOM
> <https://gerrit.wikimedia.org/g/mediawiki/extensions/Echo/+/HEAD/modules/ext.echo.init.js#46>
> for a couple of items, but does so only once.
>
> Sorry that this is confusing! We could put together a narrow JS tips and
> tricks page and link to that from the linter, but most of these have been
> fixed over the years since we introduced this in Wikimedia production code
> so there's not been much call.
>
> J.
> --
> *James D. Forrester* (he/him <http://pronoun.is/he> or they/themself
> <http://pronoun.is/they/.../themself>)
> Wikimedia Foundation <https://wikimediafoundation.org/>
> _______________________________________________
> MediaWiki-l mailing list -- mediawiki-l@lists.wikimedia.org
> List information:
> https://lists.wikimedia.org/postorius/lists/mediawiki-l.lists.wikimedia.org/
>


--
WikiWorks · MediaWiki Consulting · http://wikiworks.com