Problem/Motivation
As mentioned in the parent issue #3238306: [META] Where possible, refactor existing jQuery uses to vanillaJS to reduce jQuery footprint, we are working towards reducing our jQuery footprint. One of the ways to accomplish this is to reduce the number of jQuery features used in Drupal core. We have added eslint rules that identify specific features and fail tests when those features are in use.
There are (or will be) individual issues for each jQuery-use eslint rule. This one is specific to jquery/no-trigger
, which targets the jQuery trigger function.
Steps to reproduce
Proposed resolution
Remaining tasks
- In
core/.eslintrc.jquery.json
Change"jquery/no-trigger": 0,
to"jquery/no-trigger": 2,
to enable eslint checks for uses of jQuery.trigger()
. With this change, you'll be able to see uses of the undesirable jQuery feature by runningyarn lint:core-js-passing
from thecore
directory - Add the following lines to
core/scripts/dev/commit-code-check.sh
so the DrupalCI testing script can catch this jQuery usage on all files, not just those which have changed
# @todo Remove the next chunk of lines before committing. This script only lints # JavaScript files that have changed, so we add this to check all files for # jQuery-specific lint errors. cd "$TOP_LEVEL/core" node ./node_modules/eslint/bin/eslint.js --quiet --config=.eslintrc.passing.json . CORRECTJQS=$? if [ "$CORRECTJQS" -ne "0" ]; then # No need to write any output the node command will do this for us. printf "${red}FAILURE ${reset}: unsupported jQuery usage. See errors above." STATUS=1 FINAL_STATUS=1 fi cd $TOP_LEVEL # @todo end lines to remove
Add the block about 10 lines before the end of the file, just before
if [[ "$FINAL_STATUS" == "1" ]] && [[ "$DRUPALCI" == "1" ]]; then
, then remove it once all the jQuery uses have been refactored. - If it's determined to be feasible, refactor those uses of jQuery
.trigger()
to use Vanilla (native) JavaScript instead.
Files to fix | Usage count
- core/misc/ajax.js (5)
- core/misc/details.js (1)
- core/misc/dialog/dialog.ajax.js (5)
- core/misc/dialog/dialog.jquery-ui.js (1)
- core/misc/dialog/dialog.js (4)
- core/misc/dialog/dialog.position.js (1)
- core/misc/dialog/off-canvas/js/off-canvas.js (2)
- core/misc/displace.js (1)
- core/misc/form.js (3)
- core/misc/machine-name.js (1)
- core/misc/states.js (2)
- core/misc/tabbingmanager.js (5)
- core/misc/tabledrag.js (4)
- core/misc/tableresponsive.js (2)
- core/misc/tableselect.js (2)
- core/misc/vertical-tabs.js (1)
- core/modules/contextual/js/contextual.js (1)
- core/modules/editor/js/editor.admin.js (3)
- core/modules/editor/js/editor.dialog.js (1)
- core/modules/editor/js/editor.js (1)
- core/modules/field_ui/field_ui.js (3)
- core/modules/file/file.js (3)
- core/modules/filter/filter.js (1)
- core/modules/media_library/js/media_library.click_to_select.js (1)
- core/modules/media_library/js/media_library.ui.js (3)
- core/modules/media_library/js/media_library.view.js (1)
- core/modules/menu_ui/menu_ui.js (3)
- core/modules/node/node.preview.js (1)
- core/modules/settings_tray/js/settings_tray.js (5)
- core/modules/system/js/system.date.js (1)
- core/modules/system/js/system.js (1)
- core/modules/text/text.js (1)
- core/modules/toolbar/js/toolbar.js (3)
- core/modules/toolbar/tests/src/Nightwatch/Tests/toolbarTest.js (4)
- core/modules/tour/js/tour.js (1)
- core/modules/views_ui/js/ajax.js (2)
- core/modules/views_ui/js/dialog.views.js (1)
- core/modules/views_ui/js/views-admin.js (6)
- core/tests/Drupal/Nightwatch/Tests/tabbableShimTest.js (2)
- core/themes/claro/js/details.js (1)
- core/themes/claro/js/nav-tabs.js (1)
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3239127
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #7
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedSo part of this issue is about having a linting rule put in place to ensure that new code added to Drupal does not implement jQuery's .trigger method AND the same rule provides a nice checklist of all the places where Drupal core is currently implementing that method. That's a good piece of foundational / platform support for ensuring we're reducing our jQuery footprint.
That's part of, but not all of, the main goal here. The main goal is to actually reduce jQuery's footprint by removing all the instances of the method. Once that's done, we can close the door on adding new code that uses that part of jQuery. Then we're done.
In my opinion, we are currently unblocked from making progress in that area. So i'm going to hack on this idea a bit.
Comment #8
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedCurrent output of yarn lint:core-js-passing when the jquery/no-trigger rule is enabled:
Comment #9
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedComment #10
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedTo fix, this guidance from stack overflow seems important: https://stackoverflow.com/questions/5658849/whats-the-equivalent-of-jque...
In this article, they (as as the linting error response) recommend using JavaScript's dispatchEvent method on the targeted element. What the stack overflow article mentions is that we can avoid potential problems by being thoughtful about what kind of Event we dispatch. Considering the careful creation and use of JavaScript Events, we are likely to discover that each use of .trigger('specific_event') has a sensible default replacement.
Let's create a list of all the different kinds of events we're currently triggering and figure out a
dispatchEvent
equivalent.Comment #11
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedFound .triggers (excluding 3rd party js)
A lot of these seem to be custom events. jQuery enumerates it's events as:
Comment #12
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedWhen we use jQuery's list of events to reduce our list of found usages of .trigger we can reduce the list to:
Tricky bit might come into play when we need to stop using .trigger for jQuery events that we are overriding / extending. Best place to see what I mean here is to look at ajaxSuccess (all the ajax* "Events")
from ajax.es6.js, line 1768
Perhaps that's a good place to start. We can itemize which usages have simple fixes and which have complex fixes.
Also, from @bnjmnm in Slack:
Comment #13
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedScanning core for all instances of $.extend returns > 50 files. Perhaps that's not a good search at the moment. Maybe we should take a stab at fixing these "easy" replacements and see how much is left aftwards.
To that end, how can we map the current "easy" uses .trigger to non-jquery solutions. In order to understand better how to provide developers a good replacement for what they currently have available with jQuery's event system let's focus on Focus.
Possible deprecation code comment / guidance we could provide
Using an event object instead of doing something like $element.focus() allows the event system to properly bubble / propagate the event through the system.
When we use an Event object, JavaScript's event dispatcher triggers the event, allows any event handlers to do their actions, and when they're all done, finally focuses on the targeted element.
When .focus() is used, the targeted element immediately receives focus then the event handlers get notified of the event and they do their actions.
Comment #14
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedMore research also uncovered a possible solution for the CustomEvent concern. Regarding:
Even when we're using only JavaScript to define events we'll need to allow those events to define custom data to consume. We can do so using an Event's detail property:
Comment #15
cosmicdreams CreditAttribution: cosmicdreams at Nerdery for Nestle Purina PetCare - United States commentedConsidering all of the above, it appears that we're unblocked here. All this groundwork is all the time I have today to explore this question. I'll see if I can get back to this tonight to start hacking on a MR for some of these easy bits.
Comment #16
eelkeblokMaybe this is a silly question, but is this backwards compatible? How do jQuery events compare to events triggered through dispatchEvent()?
Comment #17
Sam Phillemon CreditAttribution: Sam Phillemon as a volunteer and at QED42 commentedcurrently looking into this
Comment #21
Sam Phillemon CreditAttribution: Sam Phillemon as a volunteer and at QED42 commentedI am facing issues with the PHP unit tests. I have attempted to fix some of the issues, but many remain, and some solutions are causing additional problems. It would be great if someone else could have a look at it.