Updating morphdom to v2.1.0

[kristoferjoseph]

Fixes #41

[yoshuawuyts]

I'm curious about the size changes for this patch

[kristoferjoseph]

hmmm, did morphdom change in size significantly? ( checking )

[kristoferjoseph]

Looks like size of morphdom is identical from v1.4.6 to v2.0.1 dist is 21k and 9k minified in both cases. Are you seeing anything troubling, or just curious about the new size?

[yoshuawuyts]

Are you seeing anything troubling, or just curious about the new size?

Oooh, nice! No I just hadn't checked out the numbers haha. Tests seem to pass so all thumbs up for getting this in :sparkles:

[kristoferjoseph]

This PR also allows a user to implement their own onBeforeElUpdated that will be called after the yo-yo pass. Seems like a nice thing to do in order to give users access to the same underlying morphdom api

[kristoferjoseph]

btw morphdom v2.1.0 is a tiny bit bigger: 25k umd'd 10k umd'd + minified

[shama]

One note about the update-events module, that list was originally intended to be a list of common events we'd want to copy over rather than a comprehensive list. Each of those incurs a perf hit, so IMO it would be better to keep that list small and let users copy any uncommon events themselves. (I know the list has grown but just want to be sure it doesn't keep growing into a comprehensive list of all events)

[kristoferjoseph]

@shama I figured as much. Felt like pulling it out would in a way "lock" it at the current state so that discussions could be had about additions etc. Was actually working on a way to only grab the events that are being used by the element in question instead of looping over a list. Wondering, do you have any thoughts on that?

[shama]

@kristoferjoseph That would be really cool. I couldn't figure out how originally and why we ended up with the list. Thanks!

[yoshuawuyts]

@shama if there's nothing holding this patch up, could we merge it? Upstream for choo, people are quite excited to use the .isSameNode() property from morphdom@2.1.0 :sparkles: - thanks!

[kristoferjoseph]

@shama I looked into only copying the events used by an element. Maybe I can chat with @patrick-steele-idem about where to get access to events in the morphdom structure. Even though @yoshuawuyts's nanomorph doesn't seem to be quite ready for primetime, it looks promising. It exposes the events as keys of the element.

[patrick-steele-idem]

I'm curious, where are you all getting the size numbers for morphdom? Here's what I am seeing:

• morphdom@2.0.0: 1.55KB gzipped (3.78KB uncompressed)
• morphdom@2.1.0: 1.75KB gzipped (4.11KB uncompressed)

(I used https://closure-compiler.appspot.com)

The size did go up, but the gzip difference is pretty minimal. we could do various things to make the code smaller, but I don't think it will make much of a difference and some tricks will make the code slightly slower. If you have any suggestions on how to improve morphdom then please share.

If you need access to any properties on a DOM element you can get access to the raw DOM nodes via the onBeforeElUpdated hook.

I'm open to a group chat if you guys think it would be helpful. Just let me know.

[kristoferjoseph]

@patrick-steele-idem I was looking at worst case scenario. ( UMD version not gzipped since this is what new users will most likely first encounter from a download link. ) SO yes, a tiny bit bigger all around, worst case scenario ( script included from download ) still small, but tiny when bundled and gzipped.

I am using the onBeforeElUpdated hook. Basically wanting to find where events on an element ( onclick, onfocus etc. ) are stashed so they can be copied over to new elements correctly. This seems like something that would be an AWESOME addition to morphdom, that and copying input values so they aren't lost during an update pass. Basically eliminating code from yo-yo

Thanks for the response. Really loving morphdom so far, let's chat about how I can help. 😸

[kristoferjoseph]

OK, looked into copying events a bit more. What I am seeing is that events are inherited properties of the to and from elements passed to onBeforeElUpdated, which is why I wasn't seeing them. This makes sense, but it would be ideal to have them as "own properties" of these elements so we aren't forced to for in loop over the entire 200+ properties. That is the one benefit nanomorph has over morphdom currently.

[patrick-steele-idem]

I haven't taken a deeper look at yo-yo/bel, but would it be possible to introduce special handling for event listeners attributes? For example, when assigning event listeners to a DOM node could you put them in a special holding area instead of attaching them as properties of the DOM node directly (e.g. domEl._belEvents = { ... }). That might simplify how event handlers are copied over. I could be completely off, but just wanted to throw that out there.

Another thing that I will throw out there is that you might want to consider rendering to a virtual DOM that is compatible with morphdom (basically just need to implement a subset of the real DOM API). This will give you some more flexibility. Reference: https://github.com/patrick-steele-idem/morphdom/blob/master/docs/virtual-dom.md

[kristoferjoseph]

Both interesting ideas. I am inclined to just encourage users to pass a event options {events:['onclick']} rather than taking on extras or needing to include virtual DOM. The beauty of this approach imho is that we get to work with real DOM elements and very little "library" code.

[yoshuawuyts]

@patrick-steele-idem nope, feel that won't work for nanomorph - we want interop with regular dom nodes so for all intents and purposes bel nodes === DOM nodes - We definitely want to evade special casing all that stuff as it'd break compat with non-bel nodes

TBH I'm not sure anymore what's going on in this thread. I'd love to have morphdom@2.1.0 land and nothing else so we can start using it upstream. Everything else seems super interesting, but I'm sure we can discuss it after morphdom lands. Hope folks can understand where I'm coming from. Thanks!

[kristoferjoseph]

@yoshuawuyts agree 💯 I will open another issue for discussing copying of events. Not sure what needs to happen to get this PR merged, but I am willing to make changes to help it along. 🙏

[brokenalarms]

Hi, I'm a little confused by where the curried onBeforeElUpdated functionality went from this PR? Is it still coming in another one?

Saw it and was exactly what I needed, since by adding our own custom onBeforeElUpdated function for memoizing sub-components, I lost the ability to transfer events. I need both, don't make me choose :P.

(Personally I'd also have the user-supplied onBeforeElUpdated run before your event one, since if we just want to return false for the comparison, then the events check becomes redundant so is a bit of a waste, but it's NBD).

I've seen the memoization example from the "choo stateful playground" example (though we aren't using choo directly), but would rather have the isSameNode check internalized once in our main node comparison to recognize our own sub-components (which was made possible by the code in this PR), rather than needing to include, e.g., svg.isSameNode = () => true; externally at the root of every one of our memoized sub-components after it first updates.

Thanks!

[kristoferjoseph]

@brokenalarms thanks for following up.

I am not sure what the reasoning was either, but it didn't seem that the currying functionality was going to be merged so I closed this.

If you think it's worth it, it would be awesome if you could please open a new issue and I can open a PR with the currying added to address it.

Thanks again 😀

[brokenalarms]

Done! Thanks 👍