Handle falsy values correctly in the dataset and style modules

[paldepind]

This PR fixes an issue where falsy values such as 0 or "" where accidentally being removed by the dataset and styles module during diffing.

ISSUES CLOSED: #303, #1093

[mreinstein]

Nice. I think this would be considered an API breaking change, right? We probably need to bump the npm major version when this lands?

[kuraga]

It was a bug, no? 😁

[mreinstein]

It was a bug, but it could be considered depended-upon behavior in the API. after this change more attributes will be present that weren't before.

[iambumblehead]

like this change 👍

[paldepind]

Thanks for the feedback.

Regarding breakage, that is a good point. We need to be careful that this is not a breaking change in any way.

The original bug reported in #303 is the following:

const vnode1 = h("i", { dataset: { foo: "" } });
const vnode2 = h("i", { dataset: { foo: "" } });
patch(elm, vnode1);
elm = patch(vnode1, vnode2).elm;
console.log(elm.dataset.foo); // is undefined but should be ""

The fact that foo disappears is a bug and not a behavior that anyone can rely on. So fixing this is not a breaking change.

The current check !dataset[name] is somewhat different from the new check !(name in dataset). While different I don't see how it will result in a breaking change. Consider for instance, a dataset with the value { foo: "bar" } getting updated to { foo: "" }. Then, with the old check, the code will actually think that foo has been removed and delete the property. Then, further on when updating properties foo will be set to "". So this change is not observable but avoids the temporary accidental removal.

[mreinstein]

The fact that foo disappears is a bug and not a behavior that anyone can rely on. So fixing this is not a breaking change.

Maybe it would be more accurate to say that "it seems unlikely this behavior is being relied upon". This incurs a change to the html that is produced (foo would be a present attribute now) and technically that could be relied upon in some other place.

[paldepind]

Yes, in theory, it could be relied upon. In this case, though, I think the buggy behavior is sufficiently unexpected and sufficiently not-useful that fixing the bug does not need to be considered a breaking change.