Warn against use of show() and hide()

[Krinkle]

Example

@Krinkle wrote at https://phabricator.wikimedia.org/T253938#6194923

The current patch uses .show() is generally stands out as an anti-pattern due to the akward legacy semantics is has to uphold. Consider the question, given an alement that has no inline style attribute, but some css classes set that make it display: none, how would jQuery know which inline style to set? Is it display: inline, display: block, display: table, display: list-item, etc. Moreover, consider a page with a <div> and a CSS rule that makes it behave as a table (like our Table of Contents), or a page with and some default display rule in your site's stylesheet. jQuery does a lot of guess work to try and get this right (involving temporary DOM elements and what not).

The caller ought to know which display is preferred, using .css('display', …). That isn't only orders of magnitude faster, it also less likely to get it wrong and is easier to debug.

Upstream

Upstream jQuery would love to get rid of or radically simplify these methods. However, what it boils down to is that when all the developer tells jQuery is "show" the element, it is surprisingly hard to do that correctly.

Further reading:

• " Everyone’s mental model is that it sets display: none in CSS […]" – blog.jquery.com, 2015
• "simplifying these methods proved to be problematic […]" – blog.jquery.com, 2016

Rule

In general, this entire area is already grey as best practice is to leave this to the stylesheet to control. That separates the concerns and also avoids interrupts that cause the browser to forcefully recompute styles. Toggling a class merely tells the browser that the next frame it will render it with those styles. Setting an inline style directly with css() also has that same benefit. However, show() and hide() come with a much higher cost.

The good news is, that one generally always knows as caller, what the intended effect is. So we'd be much better off requiring that to be done explicitly. In other words, use .css('display', 'none') to hide something and .css('display', '') to make it re-appear. Or if you know the element is hidden by default and know what you want the display value to become, then use something like .css('display', 'block').

[edg2s]

This is enforced in VE and OOUI. At the time I thought it was too disruptive to enforce globally and for most projects the performance isn't an issue. But if we can provide a useful error message this would be good to have if not just for new projects.