-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
$.fn.data isn't just for string values #5
Comments
@Canop That function has no access to the DOMs data attributes though, only new ones that are set. Unless I am missing something? |
You're missing nothing. What I meant is that when you're not interested in the string value, which is mostly when you're not interested in the data attributes but in mapping some data to DOM elements, then you can use a weak map. The code I linked to is exclusively used in that case (and shouldn't be linked to in the main page, it's just an illustration of what I mean for the purpose of this issue). In real code snippet I think you're either interested in the data-attribute (which is bad practice in my book) or in mapping dom elements to data. We should cover both cases but not with the same code (one is covered with dataset, the other one with a weak map). |
I'm tempted to just remove the jQuery data method, and make the jQuery version do |
|
Aye, but so is dataset, right? Data attributes are just regular 'ol attributes that store strings. That's all I'm trying to compare; I don't feel an urgent need to cover 100% of what jQuery does. |
I don't think anybody use |
wouldn't something like the following be suitable? myElement.data = new Map;
myElement.data.set('foo', bar); Or are you thinking of something else? |
This is a solution, but it involves adding properties to native elements, which I personally dislike and which isn't guaranteed to be safe, that's why I'm using a weak map (see first comment in this page, with link to a trivial implementation). |
That's a major difference with
dataset
.A solution would be to use a weak map to link DOM elements to maps (as is done here).
The text was updated successfully, but these errors were encountered: