search

choojs / nanomorph

🚅 - Hyper fast diffing algorithm for real DOM nodes
MIT License
726 stars 58 forks source link

The tests #79

Open tunnckoCore opened 7 years ago

tunnckoCore commented 7 years ago

Heya there! :wave:

I'm deeply digging into tests, because i'm experimenting to implement virtual dom diffing, where nanomorph guided me at most. Diffing vdom, seems pretty smaller and easy than that here.

I believe my implementation looks good, but it's just a toy for the moment. All nanomorph tests pass, except the last one for nested without id.

But while digging into test/diff.js i found one test which have opening tag, but the closing tag is missing a slash / - see it here test/diff.js#L357-L365. Is it intentional or is typo?

Also, the title isn't very clear to me. What should be the result? I think it should be <section><div></div><section>, but the title says me that it expect to be something like <section><div>'hello'</div><section> which not make sense to me to be correct result?

One more thing is to include the test suite (that file) in the npm package, so others can tests against it.

yoshuawuyts commented 7 years ago

Oops, yeah that's a typo! - thanks for catching that, PR welcome!

On Sun, Sep 10, 2017 at 1:32 PM tunnckoCore notifications@github.com wrote:

Heya there! 👋

I'm deeply digging into tests, because i'm experimenting to implement virtual dom diffing, where nanomorph guided me at most. Diffing vdom, seems pretty smaller and easy than that here.

I believe my implementation looks good, but it's just a toy for the moment. Most, if not all of the tests of nanomorph pass too :)

But while digging into test/diff.js i found one test which have opening tag, but the closing tag is missing a slash / - see it here test/diff.js#L357-L365 https://github.com/choojs/nanomorph/blob/master/test/diff.js#L357-L365. Is it intentional or is typo?

Also, the title isn't very clear to me. What should be the result? I think it should be

, but the title says me that it expect to be something like
'hello'
?

One more thing is to include the test suite (that file) in the npm package, so others can tests against it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/choojs/nanomorph/issues/79, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlevWl2UXDtxMyNTTvZ_coUS78arZLks5sg8i5gaJpZM4PSSfG .

tunnckoCore commented 6 years ago

Oooh, it's still here :laughing: i'll get that, assign please ;)