Mithril.js: Vnodes must have keys, even if the node is `null`-ish

Created on 9 Aug 2019  路  6Comments  路  Source: MithrilJS/mithril.js

Mithril version: 2.0.3

Browser and OS: Firefox 69.0b10 (64-bit)

Code

See https://codepen.io/ketan/pen/WVgPgG

Expected Behavior

There should be 2 buttons shown.

Current Behavior

There is an error: TypeError: Vnodes must either always have keys or never have keys!

In the snippet above, I'm adding 2 buttons, and a null node, which as-expected, is expected to be skipped by mithril.

However beginning mithril 2.0, it seems like mithril requires all nodes to have a key attribute, which makes no sense when the node is null-like.

Context

Some conversation about this issue on gitter: https://gitter.im/mithriljs/mithril.js?at=5d4da3d2beba830fff6310cd

Gotcha

Most helpful comment

@mvndaai No. You have two routes to take:

  • Single items: you wrap the keyed button in a fragment, like below:

```js.diff
// Based on the original pen
m.mount(document.body, {
view: function() {
return m("div", [

  • cond1 ? m("button", {key: 'one'}, "A button") : null,
  • cond2 ? m("button", {key: 'two'}, "A button") : null,
  • cond1 ? [m("button", {key: 'one'}, "A button")] : null,
  • cond2 ? [m("button", {key: 'two'}, "A button")] : null,
    null
    ])
    }
    })
- Lists of items: you use `Array.prototype.filter` to filter out the relevant entries.

```js
 // Based on the original pen
 m.mount(document.body, {
     view: function(vnode) {
         var show = vnode.attrs.show
-        return m("div", names.map(function(name) {
-            return show.indexOf(name) >= 0 ?
-                m("button", {key: name}, name) :
-                null
-        }))
+        return m("div",
+            names
+            .filter(function(name) { return show.indexOf(name) >= 0 })
+            .map(function(name) { return m("button", {key: name}, name) })
+        )
     }
 })

In general, you should avoid keys unless you absolutely need them. And even then, question your use of them. Unless you're iterating a list or conditionally recreating them in-place, keys are an anti-pattern.

I do need to make this more explicit in the docs, though.

All 6 comments

@ketan In this section, it says this:

Note that nulls, undefineds and booleans are considered unkeyed nodes. If you want the keyed equivalent, use m.fragment({key: ...}, []) which is a keyed empty fragment.

And in the v2.0.0 changelog, it has this line:

  • render: validate all elements are either keyed or unkeyed, and treat null/undefined/booleans as strictly unkeyed (#2452 @isiahmeadows)

    • Gives a nice little perf boost with keyed fragments.

    • Minor, but imperceptible impact (within the margin of error) with unkeyed fragments.

    • Also makes the model a lot more consistent - all values are either keyed or unkeyed.

So this is all working as intended. 馃檪

@isiahmeadows
I would like to request that they be skipped alltogether by mithril - without consideration for keyed or not.
I think this makes sense for the previous behavior was well suited and being that they are skipped there's no need for sort validation etc.

For example, if I am rendering a table of records, and each tr has a key and some records are not being rendered for some logic and I want it to be skipped. I am going to have all over my code .filter(a=>a).
I can easily and repeatedly run through each mapping and .filter(a=>a), but I think this should be part of the framework and it is intuitive to the developer (as the original documentation addressed)

IMHO the notion of keying null and undefined may be engineer friendly but not developer friendly.

Here's a simple example:
https://codesandbox.io/s/tabs-in-mithril-bppjl?fontsize=14

@isiahmeadows so by design you want everyone to change everywhere in their code from

bool && m(component)

to

bool ? m(component ) : m.fragment({key: "null"}, [])

I agree with @yossicadaner that falsy values should be ignored.

@mvndaai No. You have two routes to take:

  • Single items: you wrap the keyed button in a fragment, like below:

```js.diff
// Based on the original pen
m.mount(document.body, {
view: function() {
return m("div", [

  • cond1 ? m("button", {key: 'one'}, "A button") : null,
  • cond2 ? m("button", {key: 'two'}, "A button") : null,
  • cond1 ? [m("button", {key: 'one'}, "A button")] : null,
  • cond2 ? [m("button", {key: 'two'}, "A button")] : null,
    null
    ])
    }
    })
- Lists of items: you use `Array.prototype.filter` to filter out the relevant entries.

```js
 // Based on the original pen
 m.mount(document.body, {
     view: function(vnode) {
         var show = vnode.attrs.show
-        return m("div", names.map(function(name) {
-            return show.indexOf(name) >= 0 ?
-                m("button", {key: name}, name) :
-                null
-        }))
+        return m("div",
+            names
+            .filter(function(name) { return show.indexOf(name) >= 0 })
+            .map(function(name) { return m("button", {key: name}, name) })
+        )
     }
 })

In general, you should avoid keys unless you absolutely need them. And even then, question your use of them. Unless you're iterating a list or conditionally recreating them in-place, keys are an anti-pattern.

I do need to make this more explicit in the docs, though.

Can you pretty please with a cherry on top, just fix this? It seems to be making life hard for mithril users without any discernible benefit to them.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mke21 picture mke21  路  3Comments

josephys picture josephys  路  4Comments

ozgurrgul picture ozgurrgul  路  3Comments

pygy picture pygy  路  4Comments

barneycarroll picture barneycarroll  路  3Comments