Problem/Motivation
Inspired by Github's approach to removing jQuery from the front-end, add the eslint jquery plugin to Drupal core's eslint config. This will help identify and prevent new jQuery usages and make it easier to inventory existing ones.
Steps to reproduce
Proposed resolution
- Install the eslint jquery plugin that Github used for their jQuery removal efforts.
- Add rule-specific
/* eslint-disable */
declarations to files still using jQuery, so they can still pass tests - Determine if additional eslint checks are needed beyond those provided by the eslint jquery plugin, and add those as well.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
In order to limit Drupal core's dependency on jQuery for forward-compatibility with future versions, the eslint core-js-passing
config now includes the eslint-plugin-jquery plugin.
Currently, only a small number of the available rules in eslint-plugin-jquery are enabled
; these are the rules that check for jQuery features not used by core. As core eliminates uses of a given jQuery feature (typically because modern ES6 JavaScript has native replacements), additional eslint-plugin-jquery
rules can be enabled. This will prevent those jQuery features from being reintroduced.
Comment | File | Size | Author |
---|
Issue fork drupal-3191023
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 #2
bnjmnmThis adds the eslint plugin. The
/* eslint-disable */
declarations, needed for tests to pass, are applied to entire files. There are pros and cons of adding them to individual usages instead:Pros of adding to individual usages
Cons of adding to individual usages
/* eslint-disable */
declarations could still be enjoyed at a later date. If this is done after first taking care of frequently-used/easily-addressed jQuery usages, most of the cons listed above are less of a concern.So my thought is to do it on the file-level now and move to individual instances once overall jQuery use is reduced.
Comment #3
nod_That looks great :D
I'm not a fan of eslint disable in each file. we don't work file by file anymore, so we're going to fix 1 type of problem at a time. I would add the settings to legacy-passing eslint conf and disable all necessary rules instead. That way we can have an issue to get rid of the find usage or sizzle selectors, by deleting one line in the eslint config.
Comment #4
bnjmnmThis patch changes the approach to what was suggested in #3. No interdiff as that would just be noise.
Although the approach in #2 won't be used, it provided me with a way to see how many files use a given piece of jQuery functionality. A table with that information can be found here: #3179551: Provide no-library equivalents of common/useful jQuery functions.
Comment #5
lauriiiShould we have another config that enables all applicable rules for statistics in the .eslintrc.json?
Comment #6
nod_We already have 3 different eslint config, not in favor of adding another one, we could get rid of eslint.legacy.json though
Comment #8
nod_Adding another eslintrc file seems fine, after all it's under the core/ directory. And it's better to have it than not have it.
Would make tracking down jquery usage very simple (sizzle selectors especially) so +1 on this.
Still applies.
Comment #9
xjmLove it!
Can we add a change record explaining this change? It's also technically a new coding standard so gets a release note mention.
Comment #10
bnjmnmComment #11
bnjmnmComment #12
bnjmnmComment #14
bnjmnmComment #15
bnjmnmSwitching back to RTBC since the switch away from RTBC earlier was for RN/CR and those have been added + no code has changed since then.
Comment #16
xjmThanks @bnjmnm!
Tweaked the release note a little to help explain why we want less jQuery, and added the same adjustments to the CR.
Comment #17
xjmHiding patch files since there is a more recent MR.
Comment #19
xjmTested this out by adding:
Then:
Then attempted to commit that and got:
Works great!
Committed to 9.3.x and published the CR. Thanks!