Props are not reseted properly

[batiste]

I would like to know if this is a bug or is it the intended behaviour:

https://codesandbox.io/s/ppn1y5okjq

Please enlighten me about how to use snabbdom. I use it for little toy language here https://github.com/batiste/blop-language and I am running into funny bugs

[mightyiam]

I don't use the props.className. I use the class module:
https://github.com/snabbdom/snabbdom#the-class-module

[batiste]

@mightyiam hummm oki but is there an explanation for this behaviour? Should I expect the same thing for other props? This is a scary and look like a bug no?

[batiste]

I have been answered, any prop will behave like this: https://codesandbox.io/s/2x7nrlk23j

How is it possible to have this behaviour being the norm and not have massive bugs all over the place?

Maybe I misunderstand something about props?

[mightyiam]

@batiste it seems that the props module does not remove props when they are not specified. This is probably expected behavior. To "remove" a prop, set it to the empty string or whatever is correct to do.

[batiste]

I thought it was the job of snabbdom diff algorithm to do that for me.

I cannot "remove a prop" by setting an empty string because I have no idea which node is what and frankly in my app they are completely different VNodes that have nothing in common with each other. But snabbdom seems to have decided to recycle those VNode because they are similar or have more or less the same position in the tree... But then it doesn't reset the props?

I am just very surprised I am the first to run into those issues as it seems such a basic need...

[batiste]

Also if I read this code right:

https://github.com/snabbdom/snabbdom/blob/master/src/modules/props.ts#L16-L20

It seems that the old props should be deleted from the element itself? But it doesn't quite work?

[batiste]

Humm I think I should be using the attribute module instead. The prop is something quite different.

[blikblum]

This will work:
h("div", { props: { className: "" } }, "expected black"),

className is a special property, it cant be deleted

[batiste]

@blikblum oki but look at id for example here: https://codesandbox.io/s/2x7nrlk23j

This is the same thing. Id is not deleted.

[blikblum]

id is also special

[mightyiam]

Regarding recycling of VNodes, perhaps the key property could help.
https://github.com/snabbdom/snabbdom/blob/master/README.md#key--string--number

[batiste]

I mean the key does help but it is a ridiculous requirement. I will not add a Math.random() on each elements so the props are properly reseted...

I got the behaviour I wanted using the attributes module though. Props seems more about storing data onto the VNodes? I am not quite sure what is the fundamental difference between the 2 modules as they seems to cover more or less the same needs but in a different way.

@blikblum what about href?
https://codesandbox.io/s/409513z0zx

Every-time I try a different prop, the behaviour is the same. Is there really a prop that is properly reseted on the Native DOM? If there is a prop that behave "properly" I would be glad to be pointed to one.

[blikblum]

These native dom properties probably are defined as accessors so could not be deleted

Maybe this should work:

for (key in oldProps) {
    if (!props[key]) {
      (elm as any)[key] = undefined
      delete (elm as any)[key];
    }
}

I suggest doing a PR with a test if it fix your problem

[batiste]

With something like (elm as any)[key] = undefined you end up with <div id='undefined'>. AFAIK the only proper way to remove an attribute from a DOM node is to use the removeAttribute method.

[blikblum]

So you should use attrs module not props

[batiste]

@blikblum I agree but then why is the prop module is advertised in the README? Who is using it and for what purpose?

[batiste]

Here is a PR #416

[mightyiam]

#624