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

Steps to reproduce

Proposed resolution

Remaining tasks

  • In core/.eslintrc.jquery.json Change "jquery/no-ready": 0, to "jquery/no-ready": 2, to enable eslint checks for uses of jQuery .ready(). 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
  • If it's determined to be feasible, refactor those uses of jQuery .ready() to use Vanilla (native) JavaScript instead.

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3238915

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hooroomoo created an issue. See original summary.

hooroomoo opened merge request !1254

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.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented
Issue summary: View changes
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented
Status: Active » Needs review

Need to change the target to the 9.4.x branch

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented
Status: Needs review » Needs work

nod_ opened merge request !1581

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia 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.

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

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

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

Bhanu951’s picture

Bhanu951 CreditAttribution: Bhanu951 as a volunteer commented
Status: Needs work » Needs review
Issue tags: -Needs reroll

Removed Color Module and Quick Edit Module Tests that were present in earlier commit. Also remove .es6.js files.

Need to update Target Branch to 10.1.x for MR inorder to check merge status. Anyone with edit access pls update target branch in MR. Thanks.

Bhanu951 opened merge request !2975

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented

updated your MR. You had the generated file from 9.x branch replace the source file from 10.x branch. that's why a new patch is easier when going from 9.x to 10.x for JS patches :)

nod_ closed merge request !1581

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Acquia commented

The gitlab comments are absolute mayhem for a 1 line change 😂

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Could this get a test? To show that the error is being caught correctly?

mstrelan’s picture

mstrelan CreditAttribution: mstrelan at PreviousNext commented

@smustgrave do you mean we should be testing that eslint is working? I'm not sure that's necessary. Do we do the same for phpstan?

Otherwise the actual code change here is just a refactor, it should already have coverage.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Do we have any coverage for code-commit-check.sh ? If not or it's something we can test for I'll mark RTBC.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

Need to revert the changes to commit checks file. It's made just to force a run through all files. It's not necessary anymore

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented

Oh in that case this can be moved to RTBC once that's done.

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

Bhanu951’s picture

Bhanu951 CreditAttribution: Bhanu951 as a volunteer commented
Status: Needs work » Needs review

Setting to Need review as the review comments on MR are resolved.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer and commented
Status: Needs review » Reviewed & tested by the community

All good now, thanks!

  • bnjmnm committed e9abdb6f on 10.1.x 
    Issue #3238915 by Bhanu951, hooroomoo, bnjmnm: Refactor (if feasible)...

  • bnjmnm committed e3a2b4db on 10.0.x 
    Issue #3238915 by Bhanu951, hooroomoo, bnjmnm: Refactor (if feasible)...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented
Version: 10.1.x-dev » 10.0.x-dev
Status: Reviewed & tested by the community » Fixed

Looks good! Committed to 10.1.x and cherry-picked to 10.0.x.

bnjmnm closed merge request !2975

bnjmnm closed merge request !1254

Status: Fixed » Closed (fixed)

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