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-trim
, which targets the jQuery trim function.
Steps to reproduce
Proposed resolution
Remaining tasks
- In
core/.eslintrc.jquery.json
Change"jquery/no-trim": 0,
to"jquery/no-trim": 2,
to enable eslint checks for uses of jQueryjQuery.trim()
. 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
jQuery.trim()
to use Vanilla (native) JavaScript instead.
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#15 | D94x-partial_revert-3239132-15.patch | 896 bytes | joseph.olstad |
Issue fork drupal-3239132
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 #3
hooroomooComment #4
larowlanJust one question, otherwise this looks good to go
Comment #5
nod_Another duplicate trim to remove, otherwise it looks good :)
Comment #6
hooroomooComment #8
bnjmnmAll of the feedback from @nod_ and @larowlan has been addressed. I'm pleased that this was used as an opportunity to clean up some unnecessary uses as opposed to making replacements for something that was unnecessary to begin with. Those reviews were quite thorough, but I did an additional check of each
trim()
change to confirm is was the correct approach and everything looks good!Comment #9
nod_RTBC+1 by the way :)
Comment #10
larowlanCrediting nod_ and myself for reviews and bnjmnm for mentoring and review
Comment #12
larowlanFixed, thanks!
Comment #14
joseph.olstadThis change causes a javascript error as follows on the node/edit and node/add forms, possibly elsewhere:
Uncaught TypeError: callback is not a function drupalGetSummary https://localhost/core/misc/form.js?v=9.3.15:11
Solution I found is to revert this:
so change it back to :
return this[0] && callback ? $.trim(callback(this[0])) : '';
when I did this, the error went away.
Comment #15
joseph.olstadpatch
Comment #16
joseph.olstad#3291587: Regression fix for (if feasible) uses of the jQuery trim function to use vanillaJS
Comment #17
joseph.olstadI'm still seeing this regression in D10.0.10
#3291587: Regression fix for (if feasible) uses of the jQuery trim function to use vanillaJS
Comment #18
joseph.olstadok I have the steps to reproduce the regression introduced by the commit made in comment #3239132-11: Refactor (if feasible) uses of the jQuery trim function to use vanillaJS,
it can be reproduced with core alone.
in your content type, allow the "Provide a menu link" option provided by the core menu_ui module.
When editing content that does not have the checkbox checked for "Provide a menu link" the
callback is not a valid callback
, it is "Not in menu"The "Not in menu" functionality comes from the core menu_ui module.
The regression is in core,
only need core to reproduce, it's not even caused by contrib.exposed by contrib module entity_translation_unified_form doing something a bit unconventional but necessary.Patch 18 works
the $.trim() function in jQuery does more than the ES6 trim() function and it's encapsulated by jQuery.
Basically the es6 trim function is not as good as the jQuery $.trim() function.
I've spent a significant amount of time investigating ways to get this working with es6 without jQuery however the approach by jQuery is radically different because the trim function takes a parameter of item we want to trim.
Not only is the approach more sophisticated with jQuery (different) it also trims more than just whitespace, it also trims tab and newline characters whereas the es6 trim function only handles whitespace.
any chained functions I tried just didn't seem to cut the mustard. The encapsulated approach by jQuery is more advanced and I haven't had a chance to figure it out.
Comment #19
joseph.olstadFixed in
#3291587: Regression fix for (if feasible) uses of the jQuery trim function to use vanillaJS