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-is, which targets the jQuery is function.

Steps to reproduce

Proposed resolution

Remaining tasks

  • In core/.eslintrc.jquery.json Change "jquery/no-is": 0, to "jquery/no-is": 2, to enable eslint checks for uses of jQuery css(). With this change, you'll be able to see uses of the undesirable jQuery feature by running yarn lint:core-js-passing from the core 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 is() to use Vanilla (native) JavaScript instead.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#26 interdiff-3238849-22-26.txt3.03 KBmstrelan
#26 3238849-26.patch28.96 KBmstrelan
#22 interdiff-20-22.txt633 bytesmstrelan
#22 3238849-22.patch28.98 KBmstrelan
#20 3238849-20.patch28.97 KBmstrelan
#12 interdiff-11-12.txt903 bytesmstrelan
#12 3238849-12.patch75.06 KBmstrelan
#11 interdiff-10-11.txt584 bytesmstrelan
#11 3238849-11.patch76.06 KBmstrelan
#10 3238849-10.patch76 KBmstrelan

Issue fork drupal-3238849

Command icon 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:

About issue forks

Comments

hooroomoo created an issue. See original summary.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Parent issue: » #3238306: [META] Where possible, refactor existing jQuery uses to vanillaJS to reduce jQuery footprint
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Title: Refactor use of jquery/no-is to use vanillaJS » Refactor use of jquery is function to use vanillaJS
Issue summary: View changes
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Title: Refactor use of jquery is function to use vanillaJS » Refactor (if feasible) use of jquery is function to use vanillaJS

hooroomoo opened merge request !1234

hooroomoo closed merge request !1234

hooroomoo reopened merge request !1234

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan made their first commit to this issue’s fork.

mstrelan’s picture

mstrelan commented
Status: Active » Needs review
StatusFileSize
new 3238849-10.patch76 KB

Tried to rebase on to 9.4.x but failed miserably. Patch attached instead.

mstrelan’s picture

mstrelan commented
StatusFileSize
new 3238849-11.patch76.06 KB
new interdiff-10-11.txt584 bytes
mstrelan’s picture

mstrelan commented
StatusFileSize
new 3238849-12.patch75.06 KB
new interdiff-11-12.txt903 bytes

bnjmnm made their first commit to this issue’s fork.

mstrelan’s picture

mstrelan commented
Status: Needs review » Needs work
mstrelan’s picture

mstrelan commented
mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

mstrelan commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Needs review » Needs work

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
StatusFileSize
new 3238849-20.patch28.97 KB

Patch for 11.x since gitlab won't let me create a new branch.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

If the CC could be updated.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
StatusFileSize
new 3238849-22.patch28.98 KB
new interdiff-20-22.txt633 bytes
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Wasn't sure how to test.

So I applied locally on 11.x and haven't encountered any issues with the change applied all day.

Let me know if there's a better way. Going to mark for now.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs review

Just a couple of questions about where we might need to add a length check of optional chaining

  1. +++ b/core/misc/entity-form.js
@@ -52,14 +52,14 @@
-            translate = $checkbox.is(':checked')
+            translate = $checkbox[0].checked

    Do we need a length check here? 

  2. +++ b/core/modules/media/js/type_form.js
@@ -26,7 +26,7 @@
+        if (!$(context).find('#edit-options-status')[0].checked) {

    do we need a length check here too? or ?.checked 

  3. +++ b/core/modules/menu_ui/menu_ui.js
@@ -18,9 +18,7 @@
+          if ($context.find('.js-form-item-menu-enabled input')[0].checked) {

+++ b/core/modules/node/content_types.js
@@ -32,7 +32,7 @@
+        if (!$(context).find('#edit-options-status')[0].checked) {

@@ -64,7 +64,7 @@
+        if (!$editContext.find('#edit-display-submitted')[0].checked) {

+++ b/core/modules/node/node.js
@@ -40,7 +40,7 @@
+        if ($optionsContext.find('input')[0].checked) {

    here too

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For #24

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
StatusFileSize
new 3238849-26.patch28.96 KB
new interdiff-3238849-22-26.txt3.03 KB

#24.1 it's already inside a length check:

if ($checkbox.length) {
  translate = $checkbox[0].checked
    ? Drupal.t('Needs to be updated')
    : Drupal.t('Does not need to be updated');
} else {
  $checkbox = $translationContext.find(
    '.js-form-item-translation-retranslate input',
  );
  translate = $checkbox[0].checked
    ? Drupal.t('Flag other translations as outdated')
    : Drupal.t('Do not flag other translations as outdated');
}

#24.2 and #24.3 how about we use the :checked selector and check the length?

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Change didn't seem to break anything!

  • larowlan committed cd9fed87 on 11.x 
    Issue #3238849 by mstrelan, hooroomoo, smustgrave, bnjmnm, larowlan:...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed cd9fed8 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Related issues: +#3291587: Regression fix for (if feasible) uses of the jQuery trim function to use vanillaJS, +#3239132: Refactor (if feasible) uses of the jQuery trim function to use vanillaJS

***EDIT***
no comment for now
***EDIT***

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I'm getting JS errors in multiple Paragraph Browser tests on line 62 around the checkbox stuff that was discussed above. Created #3393537: Javascript errors in entity-form.js when retranslate checkbox does not exist.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Related issues: +#3238849: Refactor (if feasible) use of jquery is function to use vanillaJS

another regression

nod_’s picture

nod_
Primary language French
Location Lille
commented
Related issues: -#3238849: Refactor (if feasible) use of jquery is function to use vanillaJS +#3414415: Content language and translation AJAX expansion is backwards