Hi! README.md contains:

onUnmount()
The callback which will be invoked before a component is unmounted from DOM.

OTOH, let's look at the fragment in createComponent.js:

function unmountComponent() {
    this._isMounted = false;
    this._rootNode.unmount();
    this.onUnmount();
}

So, I'm wonder what is the onUnmount() behavior? Before? After?

And at the case intention is to call it before umounting (I hope :)), I guess it's signature must include a callback (common names for it are done or next) to permit async operations, say:

function onUnmount(el, next) {
  fetch('url').then(resp => {
    use(resp);
    next(); // real unmounting is triggered by this call!
}

Please, clarify a little :)

There are no features for setting css styles as plain text, object or as a stylesheet.

.

It just stroke my mind. Have a look at this:
https://github.com/dfilatov/vidom/blob/master/lib/nodes/TagNode.js#L122

You are treating this as attributes now as I understand. So you wouldn't need this line anymore. When you do a diffing / patching and updates the attributes, you should also update the events there. And add / remove directly.

Got it? It will also give you performance gains in the iteration loop.

Further. I don't think this code snippet are needed anymore? https://github.com/dfilatov/vidom/blob/master/lib/nodes/TagNode.js#L25

I can't find any feature for dirty checking, and force updates? Is there any?
If you have a ul tag with 4 li child nodes, how are you going to update only the 3rd li child node?

,

Why are you passing DOM event handlers via attrs ?

As in the default DOM specs, we have onClick, onMouce etc. This is not supported by you, or I'm wrong?
So overriding this with DOM event handlers on attrs are going against the DOM specs. Considered having it's own Event variable?

Maybe this can give you some ideas: https://github.com/jhudson8/react-events

REACT have a lot of back and forth issues regarding their props and attributes. To closely follow the DOM standards, shouldn't it be an idea to add a props object same as for attributes?
Due to the DOM standards, there is no mixture of them both, they are separeated.

lacking support for transitions and animation. Are this going to be supported?

xmlns are not supported.

....

....

,

Hi. I can't see how you are handling boolean attributes!

-data attribute not supported

There are XSS issues in your code, and no escaping of HTML content etc. But will probably become a bigger problem for you when you do server side rendring.

seems to be an issue with insert-to-middle-without-key in diff / patch algorithm. Everything else works just fine, except this one.

Hi! Documentation says:

update()
Forces to schedule component update.

Usually the word "schedule" is used to denote asynchronous nature. What does it mean at the vidom context?

You can't create custom attributes. Example:

.attrs({foo:"bar"})

Seems there are no shadom DOM and shadow root support?

.

Hi. I just saw your new changes, and wanted to benchmark it. But I ran into a couple of issues.

When I run the benchmark with current setup, removeChild() second argument are null

Seems like for text nodes that the `_domNode' are not set? In console.log too it show me a null.

Maybe you have to adjust the benchmark setup?

Hi! At the README.md end we get:

    return vidom.createNode('div')
        .attrs({ className : 'input' })
        .children(vidom.createNode('input').attrs({ type : 'text', className : 'input__control' }));

As far as to define CSS class className is using, it's natural to conclude DOM node properties are supposed. On the other hand, the function is named as attrs. I guess, here we get naming inconsistency, as far as attributes is associated with HTML attributes rather than with DOM node properties.

Probably, the best way is to use both (with some denoted contract who win, say, the last in call chain), that is attributes and properties functions named as attrs and props respectively (as, say, is done in Trackira), and for now just to rename attrs to props.

Hi!

Except for update() I have found asynchronous mode can be used on component unmounting. Probably, I have missed other places, but these two ones are here at least :)

I guess, rejecting an intention to keep all library operation synchronous is extremely strong and important decision. Current not-trivial SPA application can have very asynchronous operation mode, and adding additional asynchronous dimension on the presentation layer can significantly complicate overall design.

Possible complication concerns update() method especially: the method can be used extremely wide, and it is impossible to predict all those places where update() call can take place :)

I haven't found solid benchmark results proving the RAF is really unconditional winner. If at some use cases it is a winner, overcomplicating will (or can) neglect any vague profit.

So, I would suggest to eliminate any asynchronous modes from the library.

Of course, it may be the case I have missed some outstanding deep and wide researches proving RAF is a must have.

attributeNS are missing.

Reference: https://github.com/joelrich/citojs/blob/master/dist/cito.js#L650-L657

....

Seems like there are no server side rendring.

How are you going to support doctype, comments and fragment?

not added support for rdfa attributes such as resource, about, property? http://www.w3.org/TR/rdfa-core/#h-s_syntax

.

,

There are no namespace for the math tag. And how are you going to support it?

if you create a svg element with a circle as it child, it will not inherit from it's parent and get same namespace. So it will not be visible.

Rendring svg element inside a div or span tag, and it will not be visible.

There are no SVG namespace either.

d

Let say you have created a virtual DOM tree, and want to trigger a event on a particular node in the tree.

How you do it?

Example "change" should be "triggered" each time you e.g. click / change it. And I can't find any use of document.activeElement, so the focus will be lost if you have 3 select and jumping from one to one?

.

.

select and select multiple seems to have issues. Values not set correctly or loaded corectly during pageload. There are some IE issues here too that you are not aware of.

Seems also to be an issue with select in a select group with multiple children.

Are this going to be fixed? And selectedIndex?

You are missing both SVG and Math namespace.
Reference: Reference: https://github.com/joelrich/citojs/blob/master/dist/cito.js#L242-L247

dataset property not supported

Hi. Notice that this lib all over have a poor performance. Use of a constructor are terrible slow. Without RAF or setTimeOut the render take 15ms. Are this going to be fixed?

Too much overhead.

Use of 'this' keyword everywhere are also a huge performance killer. And so are use of 'new' keyword too.

Hi. I can't set className at all, only "class" as a property. Sometimes I would ned to set that.

in vanila js you can do:

node.className =

I can't get this to work, only class.

See angularjs. Aria are you going to support?