Make WordPress Core

Opened 2 years ago

Closed 2 days ago

#55126 closed enhancement (maybelater)

Twenty Sixteen: Replace frontend jQuery usage with vanilla JS

Reported by: sergiomdgomes's profile sergiomdgomes Owned by: sergiomdgomes's profile sergiomdgomes
Milestone: Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch dev-feedback close
Focuses: javascript Cc:

Description

jQuery is a large library, at around 90KB uncompressed, or 30KB gzipped. To make matters worse, it's usually enqueued in the head, and thus becomes part of the critical path, delaying first paint and subsequent metrics.

Nowadays, the library is being enqueued for relatively little gain in many situations, given that the platform supports much of the functionality. Twenty Sixteen is an example of this, as it's relatively trivial to reimplement its functionality in native JS.

This ticket focuses on removing jQuery as a frontend dependency for Twenty Twelve - specifically the frontend, as for example in the Customizer jQuery is loaded anyway (and performance is a bit less of a concern there).

See also #54171 and #54172, which do the same for other themes.

Change History (14)

This ticket was mentioned in PR #2297 on WordPress/wordpress-develop by sgomes.


2 years ago
#1

  • Keywords has-patch added; needs-patch removed

Reimplement Twenty Sixteen frontend JS without jQuery, and don't enqueue it as a result.

Trac ticket: https://core.trac.wordpress.org/ticket/55126

#2 @sergiomdgomes
2 years ago

Please note that the above patch attempts to preserve IE 11 compatibility, by implementing some utility methods for missing functionality in that browser.

If we deem that IE 11 compatibility is no longer desirable (even in the context of a theme which used to be IE 11 compatible), the code can be made smaller by removing those methods.

#3 @sergiomdgomes
2 years ago

An alternative approach to preserving IE 11 compatibility is to use a wp_get_script_polyfill-based approach / like Twenty Twenty One does. Thank you for pointing me to this, @gziolo!

This would keep the code clean and would prevent polyfills / helper functions from being loaded in modern browsers, but the downside would be that it would require bumping up compatibility to WordPress 5.0.

Last edited 2 years ago by sergiomdgomes (previous) (diff)

carolinan commented on PR #2297:


2 years ago
#4

Hi, I did a quick manual test of the PR and when there is no menu setup, I am seeing the following:

functions.js?ver=20220209:41 Uncaught TypeError: Cannot read properties of null (reading 'querySelectorAll')
    at forEachNode (functions.js?ver=20220209:41:22)
    at initMainNavigation (functions.js?ver=20220209:48:3)
    at functions.js?ver=20220209:96:2
    at functions.js?ver=20220209:293:4

Once a menu is assigned to the primary menu location, the JS error is gone, but the keyboard navigation is not working: I can not open submenu items with the keyboard only.

#5 @flixos90
2 years ago

@sergiomdgomes Given that WP core has dropped IE11 support, I would think we don't need to support it here anymore. I would think WP core includes WP core themes in that definition?

Regarding your suggestion of relying on wp_get_script_polyfill, if we still wanted to be a bit more graceful here, we could potentially use it, but only if the function exists in the core version used. That would essentially mean either the site owner has to update their WordPress version when they update the theme, or they lose IE11 support, which is a tradeoff to make. I personally think that's still a half-baked solution though, and I would prefer to follow core's decision to drop support for IE.

@desrosj Can you provide any insight what the decision to drop IE support means for themes in this regard?

sgomes commented on PR #2297:


2 years ago
#6

Thank you for taking a look, @carolinan and @felixarntz! The issues you mentioned should now be fixed.

As for IE11, I'll wait a bit longer for a consensus before making any changes on that front.

#7 @sabernhardt
2 years ago

According to the post about dropping IE11 support:

No changes will be made to any of the default bundled themes as a result of this plan. No code related to IE11 support (or any other browser that may have been supported when each theme was released) will be removed from default themes.

Twenty Fifteen and earlier themes were built to support older IE versions, too, and still have the conditional statements.

#8 @sergiomdgomes
15 months ago

Hey folks! 👋 It's been a while since the last comments 😅

I'd like to revive this effort, as twentysixteen still sees a lot of usage, and it would be nice to drop jQuery on it.

@flixos90: What do you think we need to get the ball rolling again here?

@sergiomdgomes commented on PR #2297:


14 months ago
#9

Rebased on trunk.

#10 @karmatosed
6 weeks ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to Future Release

#11 @karmatosed
3 weeks ago

  • Keywords dev-feedback added; needs-testing removed

@sergiomdgomes this is still with you as assigned, are you working on this or do you wish to have it unassigned. My expectation is the later but I didn't want to remove you from it yet as you seemed keen on completing this.

We have a lot of thoughts on this and I would love to get to some resolution on the ticket over have it sat because this is quite an older theme now. What are everyone's thoughts now some time has gone by? cc @sabernhardt and @flixos90.

#12 @sergiomdgomes
3 weeks ago

Hey @karmatosed, thanks for reaching out!

I'm happy to either do further work on this or to hand it off to someone else who's keen on seeing it finished.

It doesn't seem like there's a lot of interest from the community, however, so perhaps it is best to simply drop this change and close the associated PR?

#13 @karmatosed
3 days ago

  • Keywords close added

Thank you @sergiomdgomes my own reflection is aligned with your last point that due to the older nature of this theme I would recommend in this case not going any further. That doesn't mean it couldn't be something to explore at a later date, but it does mean for now I would recommend only focusing on bugs not enhancement or foundation changes.

Aligned to that, I am going to add the close keyword as a recommendation. This absolutely could be something to discuss if doesn't fit though. Thank you everyone who has collaborated on this.

#14 @karmatosed
2 days ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from assigned to closed

I am going to close this for now, thank you everyone for your collaboration.

Note: See TracTickets for help on using tickets.