check for typeof attribute value, if is string use setAttribute#511
check for typeof attribute value, if is string use setAttribute#511enapupe wants to merge 1 commit intopreactjs:masterfrom enapupe:dom-setacessor-setAttribute-if-string
Conversation
src/dom/index.js
Outdated
| l[name] = value; | ||
| } | ||
| else if (name!=='list' && name!=='type' && !isSvg && name in node) { | ||
| else if (typeof value === 'string' && !isSvg && name in node) { |
There was a problem hiding this comment.
Think it'd be possible to invert this conditional and have String fall through to the complete attribute add/update/remove flow on line 75?
|
Looking at the travis build, it actually seems like this might be better for performance! (not sure how but I'll take it) |
|
Don't forget to consider merging #495 after fixing this one. |
|
@enapupe yup yup, I'll pair up the two. These are likely to go into |
|
I was chatting with @developit about this PR today and he suggested adding a check to see if the value being set is an object and if so, falling back to using a property setter. I tried to outline the expected behavior in this comment (sorry it's a bit long!) |
|
Since this is opened against v8, and the original issue (#492) has been closed, I'm going to close this PR. If there is a another situation where this change would be beneficial in Preact X, open a new issue with that case, and we'll take a look. |
6 participants
I wasn't able to run react tests against this change but I tried to change the logic as minimum as possible.
ref #492