domData get and set #317
domData get and set #317
Conversation
packages/metal-dom/src/domData.js
Outdated
| * @return {!Object} | ||
| */ | ||
| static get(element, name, initialValue) { |
There was a problem hiding this comment.
Hey @diegonvs,
I don't think we should remove any functionality from the get method, since that will be a breaking change (even if we account for our own use cases it might break somewhere else).
Can we maintain the initialValue argument, and maybe just call the new set method from inside get if the arg is passed?
There was a problem hiding this comment.
Good point @Robert-Frampton , i think in the get method there could be no creation or editing of domData but thinking so as not to break API, this makes sense.
There was a problem hiding this comment.
Since we're leaving the API alone for get, we should also leave all the instances of it alone in metal. For example, 41756bc should just be left using get.
| if (!element[METAL_DATA]) { | ||
| element[METAL_DATA] = {}; | ||
| } | ||
| if (!name || !value) { |
There was a problem hiding this comment.
Wouldn't this check prevent someone from passing a falsy value?
domData.set(element, 'foo', true)
// foo === true
domData.set(element, 'foo', false)
// foo === true
There was a problem hiding this comment.
I'll correct this!
|
What do you guys think about the |
|
Hey @diegonvs, I think the return !!element[METAL_DATA]; |
|
Hey @diegonvs, At some point today we'll be cutting a release. Not trying to rush you, but what's your estimate for finishing this PR? |
|
Closing in favor of #319 |
No description provided.