- First commit to issue fork.
- Status changed to Needs work
over 1 year ago 5:02am 27 June 2023 - 🇦🇺Australia mstrelan
Might need some guidance from the big brains on this one. jQuery's data function has two purposes, one for getting and setting data attributes on elements and another for getting and setting arbitrary bits of data in an internal cache for an element. It seems the former can be addressed using the dataset property (as per ajax.js) whereas the latter may can use a WeakMap object (as per tabbingmanager.js). Is this the right approach?
- 🇳🇿New Zealand quietone
Since this needs some input on direction, I am adding the 'Needs architectural review' tag and changing to NR. Added and item to the remaining tasks to get answers to those questions.
- 🇫🇷France nod_ Lille
To reply to #9 Let's first make the changes for the getting/setting data attributes using dataset, let's leave the second use case alone (like vertical tabs) because it'll need something else than a straight replacement of the jquery call (probably some new architecture for these things).
@shubh511 I think you're still working on it but please make sure to test the code before setting this to Needs review, as-is the code is not going to work, jquery doesn't have a setAttribute function. Also with data attributes let's use
element.dataset.someThing = 'value';
instead ofelement.setAttribute('data-some-thing', 'value');
- 🇮🇳India shubh511
raised a MR for this issue but the problem is that it is failing pipeline and when fixing one test case will result in failing another test case and viceversa..
- Assigned to kostyashupenko
- Issue was unassigned.
- 🇷🇺Russia kostyashupenko Omsk
There are still several existing data() methods:
% yarn lint:core-js-passing core/misc/ajax.js 1104:17 error Prefer WeakMap to $.data jquery/no-data 1111:31 error Prefer WeakMap to $.data jquery/no-data 1580:7 error Prefer WeakMap to $.data jquery/no-data core/misc/autocomplete.js 224:13 error Prefer WeakMap to $.data jquery/no-data core/misc/dialog/dialog.ajax.js 59:31 error Prefer WeakMap to $.data jquery/no-data 60:13 error Prefer WeakMap to $.data jquery/no-data 105:24 error Prefer WeakMap to $.data jquery/no-data core/misc/tabbingmanager.js 297:25 error Prefer WeakMap to $.data jquery/no-data 302:9 error Prefer WeakMap to $.data jquery/no-data 314:25 error Prefer WeakMap to $.data jquery/no-data 332:13 error Prefer WeakMap to $.removeData jquery/no-data 340:13 error Prefer WeakMap to $.data jquery/no-data ✖ 12 problems (12 errors, 0 warnings)
1. regarding ajax.js and 3 remaining errors - i think all 3 errors can be fixed, but need to research (i just don't have available time anymore for this, so skipped)
2. regarding autocomplete and dialog - there are some data methods which are still exist, which are related to the plugins itself (jquery autocomplete, jquery dialog). I didn't touch those 2 files at all, i think the better way is to wait for replacement of jquery autocomplete in core, and replacement of jquery dialog to native dialog element functionality.
3. regarding tabbingmanager - i didn't understand how to reproduce it in Drupal for making local tests. From what i saw in tabbingmanager.js - this file is fixable in terms of replacing jquery data() method
4. Also i prefer to use getAttribute() instead of dataset, coz getAttribute works seems at least 2x faster https://www.measurethat.net/Benchmarks/Show/14432/0/getattribute-vs-dataset
- First commit to issue fork.
- Status changed to Needs review
8 months ago 4:20am 25 April 2024 - Status changed to Needs work
8 months ago 4:48am 25 April 2024 The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- First commit to issue fork.
- Status changed to Needs review
6 months ago 12:18pm 3 July 2024 - Status changed to Needs work
6 months ago 2:19pm 5 July 2024 - 🇺🇸United States smustgrave
Trying to keep up with these but seems the javascript tests are failing repeatedly (re-ran 3 times)