[Moving to wikitech-l; mediawiki-l is a little off-topic.]
On Mon, 26 Jul 2021 at 21:27, Yaron Koren <yaron57(a)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…
>
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…>
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/>
Hello all,
Next week we will remove the following methods without deprecation:
- Article::delete()
- Article::confirmDelete()
- ImagePage::delete()
This is part of an ongoing refactoring of deletion-related code. The
methods above are currently bloated: beyond deleting an article, they're
responsible for building the UI, checking permissions, handling errors etc.
The relevant UI-related code is being moved to DeleteAction, so use that if
you need to do something with the UI.
Maintaining BC in the old methods is challenging, due to how many things
this code does, and how much code is being moved around. According to
codesearch, these methods are only used by a test in WikibaseLexeme (which
has been temporarily disabled). Thus, we decided to remove them without
deprecation after this week's train.
Please reply to this email if you're affected by this and don't know how to
fix.
Best regards,
--
https://meta.wikimedia.org/wiki/User:Daimona_Eaytoy
"Daimona" is not my real name -- he/him
This email is a summary of the Wikimedia production deployment of
1.37.0-wmf.18.
- Conductor: Jeena Huneidi
- Backup: Mukunda Modell
- Blocker task: T281159 <https://phabricator.wikimedia.org/T281159>
- Status: Live on all wikis <https://versions.toolforge.org/>
*📊Stats*
- 244 patches ▁▄█▅▄
- 1 rollback ▁▁███
- 1 day of delay (going to group0) ██▁▁█
- 6 blockers ▄▁▁▆█
🚂🌈
Big WikiLove to the folks who helped out:
- Timo Tijhof
- James D. Forrester
- Tacsipacsi
- Lucas Werkmeister
- Jon Robson
- RhinosF1
- KartikMistry
- abi_
- Zabe
Until the next train,
– Your local WikiMotive congregants
Could someone help me with something related to <b>gerrit.wikimedia.org</b>? My <b><a href="https://gerrit.wikimedia.org/r/dashboard/self">dashboard</a></b> has <b>6 changes</b> that need code review, which is important because they are requests from Phabricator . So could someone experienced review the code for my requested changes? Thank you in advance!
For performance sensitive tight loops, such as parsing and HTML
construction, to get the best performance it's necessary to think
about what PHP is doing on an opcode by opcode basis.
Certain flow control patterns cannot be implemented efficiently in PHP
without using "goto". The current example in Gerrit 708880
<https://gerrit.wikimedia.org/r/c/mediawiki/core/+/708880/5/includes/Html.ph…>
comes down to:
if ( $x == 1 ) {
action1();
} else {
action_not_1();
}
if ( $x == 2 ) {
action2();
} else {
action_not_2();
}
If $x==1 is true, we know that the $x==2 comparison is unnecessary and
is a waste of a couple of VM operations.
It's not feasible to just duplicate the actions, they are not as
simple as portrayed here and splitting them out to a separate function
would incur a function call overhead exceeding the proposed benefit.
I am proposing
if ( $x == 1 ) {
action1();
goto not_2; // avoid unnecessary comparison $x == 2
} else {
action_not_1();
}
if ( $x == 2 ) {
action2();
} else {
not_2:
action_not_2();
}
I'm familiar with the cultivated distaste for goto. Some people are
just parotting the textbook or their preferred authority, and others
are scarred by experience with other languages such as old BASIC
dialects. But I don't think either rationale really holds up to scrutiny.
I think goto is often easier to read than workarounds for the lack of
goto. For example, maybe you could do the current example with break:
do {
do {
if ( $x === 1 ) {
action1();
break;
} else {
action_not_1();
}
if ( $x === 2 ) {
action2();
break 2;
}
} while ( false );
action_not_2();
} while ( false );
But I don't think that's an improvement for readability.
You can certainly use goto in a way that makes things unreadable, but
that goes for a lot of things.
I am requesting that goto be considered acceptable for micro-optimisation.
When performance is not a concern, abstractions can be introduced
which restructure the code so that it flows in a more conventional
way. I understand that you might do a double-take when you see "goto"
in a function. Unfamiliarity slows down comprehension. That's why I'm
suggesting that it only be used when there is a performance justification.
-- Tim Starling
This email is a belated summary of the Wikimedia production deployment of
1.37.0-wmf.17.
Dan Duvall was the train conductor last week. I was the backup conductor.
Blocker task: https://phabricator.wikimedia.org/T281158
The new version is live on all sites: https://versions.toolforge.org/
🎶 Optional: For optimal enjoyment, queue up a fine track by They Might Be
Giants <https://s3.amazonaws.com/TMBG_pod/TMBGPodcast57a.mp3> for your
listening pleasure while you dive into these stats:
== 📊 Stats ==
* 287 patches ↓
* 16 backports
<https://phabricator.wikimedia.org/source/mediawiki/compare/?head=wmf%2F1.37…>
* 1 rollback (for ~3hours) ↑
* 0 days of delay
* 5 blockers added, 3 resolved, 2 removed ↑
** RESOLVED:
*** T287988 PHP Warning: Class __PHP_Incomplete_Class has no unserializer
<https://phabricator.wikimedia.org/T287988>
*** T288191 substituting {{#tag:ref}} tags and templates; and pipe tricks
fail in Page namespace <https://phabricator.wikimedia.org/T288191>
*** T288288 1.37.0-wmf.17 deployment moved footer from bottom of the page
to left column in Monobook <https://phabricator.wikimedia.org/T288288>
== 🚂🌈 ==
1 task was mentioned, which is not a train blocker but remains open:
* T287410 ResourceLoaderSkinModule: Skins should not use the legacy styling
feature <https://phabricator.wikimedia.org/T287410>
Thank you to everyone who helped out whether in a big way or a small one.
Special thanks goes to the owners of these phabricator usernames:
* dduvall
* Jdlrobson
* Jdforrester-WMF
* Ladsgroup
* Urbanecm
* DannyS712
* Krinkle
* Legoktm
* Tpt
* Xover
* jouncebot
* Jdforrester-WMF
Yes I know James is mentioned twice. There really aren't enough thank-yous
for all that James does.
– The Trainbow Bunch
Hello,
On Thursday 12th August at 06:00 AM UTC we'll switch our phabricator
database master to a different host (
https://phabricator.wikimedia.org/T288197)
There'll be 1 or 2 minutes of read-only time. Writes will be blocked, reads
will continue as usual.
Sorry for the inconveniences.
Manuel.