begin implementation #2
Conversation
| // Memoize a bel element | ||
| // null -> null | ||
| function cacheElement () { | ||
| function cacheElement (fn) { |
There was a problem hiding this comment.
perhaps assert()ing the function is actually a function might be useful - a pattern I like is the assert.equal call which takes two arguments + a custom error message that includes the name of the module where the error occured for better traces
| const args = Array.from(arguments) | ||
| const argsAreTheSame = JSON.stringify(store.prev) === JSON.stringify(args) | ||
|
|
||
| if (argsAreTheSame) return store.el |
There was a problem hiding this comment.
In order for this to work we can't return the el directly as a Node can't exist in two different DOM trees at the same time. Instead we need to return a proxy element as per this repo - hope this makes sense hah
| const store = {} | ||
|
|
||
| return function render () { | ||
| const args = Array.from(arguments) |
There was a problem hiding this comment.
Array.from() isn't quite compatible with most browsers yet; slicing would probably be better - though on a first render there's no need to diff I reckon so this slicing could be avoided if no prior arguments exist
|
|
||
| return function render () { | ||
| const args = Array.from(arguments) | ||
| const argsAreTheSame = JSON.stringify(store.prev) === JSON.stringify(args) |
There was a problem hiding this comment.
I suspect this might be slow for larger data sets; I think we could get away with shallow equality and triple equals. Perhaps we should allow an optional comparison function in the cacheElement call: cacheElement(createElement, compare) so that if a deep comparison or the like needs to be done it's flexible enough to be supported
|
Added some comments; hope this makes sense! |
|
@jekrb heya, how's it going? Did my feedback make sense? Is there any way I can help? |
|
Hey @yoshuawuyts! Sorry for the late response. The feedback makes loads of sense and I'm so happy you've provided it. I feel really bad with not carrying this on as quickly as I hoped initially. My day job has been taking up more of my mental capital than anticipated lately 😆 If you want to carry this on faster I won't take any hard feelings. Otherwise I'd like build this out over the weekend |
|
Yeah, I'm pushing this further a bit faster as I got a talk coming up soon and this is one of the crucial aspects that I'll be discussing soooo :D - thanks heaps for the effort you've put into this, def hope it was helpful - I def enjoyed providing feedback ^__^ Also don't sweat about not pushing it forward faster; open source is hard, and in the end what matters the most is that you enjoyed doing it C: - so cheers to that 🎉 |
|
Awesome! Yeah I totes learned a bunch and had lots of fun. Good luck on the talk! |
ref #1
I'll be getting back on the rest of this tonight! Feel free to make comments where you feel there should be improvements.