crysalead-js / dom-layer

Virtual DOM implementation.
MIT License
30 stars 1 forks source link

keys, classes, id BANG! #58

Closed ghost closed 9 years ago

ghost commented 9 years ago

Hi again! Just for fun I was randomly trying out the keys. Here is my findings:

The script will throw if

I haven't tested with Arabic language, OR Spanish and Franch but specials there too.

Should these be validated somehow??

Seems to be the same thing with id and your class too. For class I found this... https://github.com/Matt-Esch/virtual-dom/issues/249

A lot of other places probably too where this happen, but didn¨t test

jails commented 9 years ago

Would you mind providing a fiddle so I can see some code ?

ghost commented 9 years ago

Sorry, very hard for me that. And I also got hard time find a way to present problem correct to you. But I hope it was ok I used code from your spec?

You see this? Example if from and to got the same key, it will BANG!

       // NOTE!! This one will throw because 4 and 4 are equal!
        var from = buildNodes(["a", "b", "c", "4", "55"]);
        var to = buildNodes(["7", "4", "0", "2", "6"]);

       // Others

        var from = buildNodes(["1", "2", "3", "4", "55"]);
        var to = buildNodes(["-7", "4", "3", "2", "6"]);

        var from = buildNodes(["1", "2", "3", "4", "5"]);
        var to = buildNodes(["7", "4", "3", "2", "6"]);

        var from = buildNodes(["1", "2", "3", "4", "-4455"]);
        var to = buildNodes(["7", "4", "+0", "2", "6"]);

For class and id same I think if you patch them. Equal and BANG!

jails commented 9 years ago

Good catch :+1: ! But I don't think it was related to the encoding. The virtual-dom issue don't apply in dom-layer since it's related to a custom virtual-dom feature. Should be fixed in https://github.com/crysalead-js/dom-layer/commit/3cf5cea397bd4a63439107d529bb45706d79405c