Why allow transformers to replace the vElm node?

[maranomynet]

Hi. I'm curious why you allow transformer functions to replace the virtual element node:
https://github.com/barneycarroll/mattr/blob/master/index.js#L33

It looks to me like this leads to unpredictable behaviour - as the for-loop is already fixed on, and half-way through, the property names of the original vElm. And if the transformer returns a node made by a mattr enhanced function, you might even end up with double transformations on some attributes.

The safest/sanest option might be to simply exit the for-loop when a transformer returns something other than undefined, like so:

if ( transformed !== undefined ) {
    node = transformed;
    break;
}

Or am I missing something obvious?

[barneycarroll]

Replacement is useful for all sorts of things — you can change element type, conditionally not render the node… you could even use it to conditionally just render the children (like your proposal for empty selectors).

The idea that one transform might modify the node in a way a subsequent transform doesn't expect is well pointed out but not limited to this particular aspect; say for example I have a lazyload custom attribute which replaces the node with a span and listens for scroll to see if the element is in view, then replace it with an image tag, and another responsive plugin which takes a map of pixel widths to different resolution image sources. One will necessarily execute before the other. There is no specified order, yet it's clear the transforms will need to take account of each-other to avoid chaos.

In all problem cases, there's no procedural solution — conflicting transforms need to have oversight if user expectations are to be met. So ultimately any errors that come out of this are 'user error'.

[maranomynet]

Aha, I like the use-cases you mention.

But I still think the transformations function should just say "Woah, the transformation returned a new node, which most likely is already transformed - so it makes no sense for me to continue!" and break the loop. :-)

[barneycarroll]

Yeah to be honest when I read your proposal it made immediate obvious sense — I was going to ask you to file a pull request, but then by talking through the root of the problem I realised it adds code without solving any problems.

The truth is there's no clear user expectation for a transform not to run: if anything, breaking the loop is a violation of user expectation. The scenario where the user puts several custom attributes on a node and one returns a new node does not necessarily mean the others shouldn't run (in fact object iteration order differs between Javascript engines, so in one browser the replacing transform may occur 1st, cancelling further transforms, while in another it could run last). So adding this behaviour would only add more possibilities for confusion.

I guess this minefield should be documented, with the advice that users should take care to avoid composing nodes with potentially contradictory custom attributes — and, where they really do want the behaviour of several custom attributes which could lead to different outcomes depending on what order they execute in, to compose new attributes that invoke the others in the desired order…

[maranomynet]

OK, fair enough.

It's a bit too much of a minefield (and too difficult to explain) for my taste :-) so I've made my personal implementation exit on replacement.

That only requires documenting a warning that transformations shouldn't perform instant actions/setup outside of the vElm that depend on the formal config and ctx.onunload lifecycle events occurring.