Support Lazy nodes from V2 #16
Conversation
src/index.js
Outdated
|
|
||
| if (node != null && typeof node !== 'boolean') { | ||
| if (node.type === 2 && node.lazy) { |
There was a problem hiding this comment.
Are you sure we need both checks, can't we leave only one?
There was a problem hiding this comment.
I was thinking about it myself and decided to take a defensive approach here. I couldn't think of a scenerio where only one of the 2 would be present though. If we're to leave only one do you prefer the node.type === 2? You already have node.type === 3 check in the code.
src/index.js
Outdated
| if (node.type === 2 && node.lazy) { | ||
| node = node.lazy.view(node.lazy) | ||
| } |
There was a problem hiding this comment.
Shouldn't this part be inside resolveNode function so it will resolve any Lazy components recursively?
Would be cool to have a test for nested Lazy.
There was a problem hiding this comment.
Good catch! This place didn't feel right to me. It's so much cleaner in the resolveNode function. Will send an updated PR in a sec.
There was a problem hiding this comment.
Totally agree on the test for the nested lazy! I will add it.
There was a problem hiding this comment.
I've just checked and interestingly HA2 doesn't support nested lazy. But since it doesn't cost us anything to add it here I'd leave the nested lazy support.
There was a problem hiding this comment.
Actually there was a problem not just with nested Lazy({ render: Lazy({...}) }) but also with using functional components inside Lazy view Lazy({ render: Component }).
I think we are good now. I will create a new release later today. Thank you!
|
Let me know if you have any comments to the new version. |
|
Just released under v3.2.0 🎉 thanks again. |
Types of changes
Checklist: