-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating morphdom to v2.1.0 #44
Conversation
I'm curious about the size changes for this patch |
hmmm, did morphdom change in size significantly? ( checking ) |
Looks like size of morphdom is identical from |
Oooh, nice! No I just hadn't checked out the numbers haha. Tests seem to pass so all thumbs up for getting this in ✨ |
This PR also allows a user to implement their own onBeforeElUpdated that will be called after the |
btw morphdom v2.1.0 is a tiny bit bigger: |
One note about the |
@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. |
@kristoferjoseph That would be really cool. I couldn't figure out how originally and why we ended up with the list. Thanks! |
@shama if there's nothing holding this patch up, could we merge it? Upstream for |
@shama I looked into only copying the events used by an element. |
I'm curious, where are you all getting the size numbers for
(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 If you need access to any properties on a DOM element you can get access to the raw DOM nodes via the I'm open to a group chat if you guys think it would be helpful. Just let me know. |
@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. ) I am using the Thanks for the response. Really loving |
OK, looked into copying events a bit more. What I am seeing is that events are inherited properties of the |
I haven't taken a deeper look at Another thing that I will throw out there is that you might want to consider rendering to a virtual DOM that is compatible with |
Both interesting ideas. I am inclined to just encourage users to pass a event options |
@patrick-steele-idem nope, feel that won't work for TBH I'm not sure anymore what's going on in this thread. I'd love to have |
@yoshuawuyts agree 💯 |
Hi, I'm a little confused by where the curried Saw it and was exactly what I needed, since by adding our own custom (Personally I'd also have the user-supplied I've seen the memoization example from the "choo stateful playground" example (though we aren't using choo directly), but would rather have the Thanks! |
@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 😀 |
Done! Thanks 👍 |
Fixes #41