Skip to content
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

m.render() ignores the key of the vnode returned by comnponents #2654

Closed
barneycarroll opened this issue Jan 15, 2021 · 3 comments
Closed

m.render() ignores the key of the vnode returned by comnponents #2654

barneycarroll opened this issue Jan 15, 2021 · 3 comments
Assignees
Labels
Area: Core For anything dealing with Mithril core itself Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Gotcha For unwanted behavior that is still correct under the given circumstances

Comments

@barneycarroll
Copy link
Member

I keep running into this gotcha whereby I have a solitary dynamically-keyed node as the return value of a components view, and changing the vnode key doesn't cause the node to reinitialise. The reason this doesn't work is that AFAICT component instances are the only case in which a user-supplied vnode isn't normalised as a list item. Just as m(tag, ...vnodes) is equivalent to m(tag, [...vnodes]) & m.render(el, vnode) to m.render(el, [vnode]), I suggest we normalise component view output such that instances are always fragments - which as well as obviating this edge case would have the added benefit of making vnode tree traversal easier to reason about.

Reproduction here.

I'm happy to implement this myself.

@barneycarroll barneycarroll added the Type: Bug For bugs and any other unexpected breakage label Jan 15, 2021
@pygy
Copy link
Member

pygy commented Jan 16, 2021

I'd rather check for the presence of a key at diff-time, it will have the same ultimate result, and will be more efficient.

@pygy pygy added Area: Core For anything dealing with Mithril core itself Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Gotcha For unwanted behavior that is still correct under the given circumstances labels Jan 16, 2021
@pygy pygy changed the title Ensure component instances are always fragments for universally consistent keys behaviour m.render() ignores the key of the vnode returned by comnponents Jan 16, 2021
@pygy pygy removed the Type: Bug For bugs and any other unexpected breakage label Jan 16, 2021
@barneycarroll
Copy link
Member Author

I still think my suggestion is better because along the lines of #2669 it moves us towards monomorphic vnode structures which are more predictable. The counter proposal gets us into the same category of issues as #2669 where we introduce extra code paths for efficiency when IMO having a more consistent structure makes the codebase more maintainable and has the added benefit of making tools integrating with vnode easier to write.

@barneycarroll
Copy link
Member Author

Closing this, keeping #1713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core For anything dealing with Mithril core itself Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Gotcha For unwanted behavior that is still correct under the given circumstances
Projects
Status: Closed
Development

No branches or pull requests

3 participants