"key" attribute can be used outside of iteration

[nolanlawson]

Description

During refactoring (#2677), we found a test in a downstream that broke because of an odd situation:

<!-- foo.html -->
<template>
  <x-bar key={foo}></x-bar>
</template>

key is intended to be used inside of an iteration. But it turns out, there's nothing to stop you from using it wherever you want. And if you do, you can create dynamic keys where the engine doesn't expect it.

For the above example, the compiler generates:

function tmpl($api, $cmp, $slotset, $ctx) {
    const {k: api_key, c: api_custom_element} = $api;
    return [api_custom_element("x-child", _xChild, {
        key: api_key(0, $cmp.color)
    })];
}

Whenever this template rehydrates, it will go through the updateStaticChildren path, even though the vnode key may change every time.

Steps to Reproduce

<template>
    <x-child key={somethingThatMayChange}></x-child>
</template>

Version

LWC 2.9.x

Possible Solution

Only use the key attribute inside of an iteration.

[uip-robot-zz]

This issue has been linked to a new work item: W-10713835

[pmdartus]

This is an interesting bug. I don't know if it's a conscious decision the component developer made to trick the engine. Effectively if the key changes between render cycles the engine will be forced to unmount the existing component and create a brand new one.

Ideally, it's something the template compiler should prohibit. But to avoid backward compat issues, I would suggest that the compiler ignores the key directive when the parent element doesn't have a for:each or iterator:* directive.

---

I never took the time to properly formalize this, but I think that the current key directive is broken. When using for:each directive on a <template> we currently require all the children elements to have a key directive.

<template>
  <template for:each={items} for:item="item">
    <div key={item.id}>{item.name}</div>
    <span key={item.id}>{item.value}</span>
  </template>
</template>

The key directive is used to efficiently diff dynamic children. This enables the LWC engine to re-order the items in the list and avoid recreating the entire list when an item is appended at the beginning. The engine treats the VNodes produced by the iteration as a single list of VNodes, which is incorrect. In the previous example, the engine diffs a list of alternating <div> and <span> VNodes. However all the VNodes are not equal, a <div> is always followed by a <span> generated by the same item.

[
  { type: VNodeType.Element, sel: 'div', key: 'foo-0' },
  { type: VNodeType.Element, sel: 'span', key: 'foo-1' },
  { type: VNodeType.Element, sel: 'div', key: 'bar-0' },
  { type: VNodeType.Element, sel: 'span', key: 'bar-1' },
  // [ ... ]
]

The correct approach would be to treat each item produced by the iteration as a VNode fragment, where each fragment can be reordered. In this approach, the fragment itself would have a key.

[
  { 
    type: VNodeType.Fragment, 
    key: 'foo',
    children: [
        { type: VNodeType.Element, sel: 'div', key: '0' },
        { type: VNodeType.Element, sel: 'span', key: '1' },
    ]
  },
  { 
    type: VNodeType.Fragment, 
    key: 'bar',
    children: [
        { type: VNodeType.Element, sel: 'div', key: '0' },
        { type: VNodeType.Element, sel: 'span', key: '1' },
    ]
  }
  // [ ... ]
]

This brings me to the actual proposal. Instead of asking developers to add key directive to for:each children elements, I think it would make more sense to ask developers to add the key directive on the same element than the for:each:

<template>
  <template for:each={items} for:item="item" key={item.id}>
    <div>{item.name}</div>
    <span>{item.value}</span>
  </template>
</template>

This is effectively what other template-based frameworks are doing:

• Vue: :key
• Svelte: #each
• Angular NgFor: trackBy

I think it's something we can achieve while keeping backward compat. IMO, the template compiler can safely assume the key assigned to the first child element could be reused on all the children elements.

[nolanlawson]

I like this proposal. I would also suggest we name it lwc:key, since (AFAICT) key is the only compiler directive we have that doesn't have a namespace : in it. That would also help emphasize that it's different from the old key attribute.

For the record, I looked into this particular component, and the key was added by mistake during a refactoring. It wasn't intentional.

[nolanlawson]

I think in the short-term, a good non-breaking solution would be to make key a no-op when the parent element doesn't have a for:each or iterator:* directive. (I.e. it doesn't cause an api_key() call). I can't imagine how this would break anyone.

[nolanlawson]

Fixed in #2699