search

marcelklehr / vdom-virtualize

Virtualize a DOM node
MIT License
131 stars 27 forks source link

Test cases #21

Closed bitinn closed 9 years ago

bitinn commented 9 years ago

Hi @marcelklehr, I migrated test cases from https://github.com/TimBeyer/html-to-vdom to this repo. Could you take a look?

This is also partly due to a discussion we are having about using virtual-dom for progressive enhancement (at least I would like to get them working). ref: https://github.com/Matt-Esch/virtual-dom/issues/227#issuecomment-99420341

Changes I have made:

  1. adding test cases from html-to-vdom, do note my comments in the test file, innerHTML behaves differently from htmlparser2, so there are difference between vdom-virtualize and html-to-vdom.
  2. also provide a npm run dist command to build bundle.js
  3. .gitignore file

You should be able to run npm test after a npm install, it will open a localhost to run test cases in your default browser. I tried to get Testling CI (cross-browser tests) working but they API appear to be broken since 2015, so that needs more work.

bitinn commented 9 years ago

The main difference between vdom-virtualize and html-to-vdom, besides the parser, is that virtualize wrap text node with a vnode in output, as you can see in the first 2 tests:

https://github.com/bitinn/vdom-virtualize/blob/test-cases/test/index.js#L11

Not sure which is preferred, love to hear @TimBeyer 's opinion.

bitinn commented 9 years ago

I should also note that I modified some of the tests to better reflect how browser dom work:

eg. if you set shorthand style background: red, it will be expanded to background-color: red and other background properties, this is a potential gotcha. (obviously js-based html parser like htmlparser2 won't do this because they are not fully CSS aware like browsers)

TimBeyer commented 9 years ago

@bitinn In my opinion, the right approach is clearly creating only a TextNode in case there is text only.
If the chain html -> vdom -> html does not create the same output as it got input, I consider the implementation broken. That clearly would be the case for something like
Just Text -> VNode(VText('Just Text')) -> <div>Just Text</div>.

marcelklehr commented 9 years ago

mhpf, closing this wasn't my intention of course...

bitinn commented 9 years ago

please see my updated PR #29