Wrong components rendered by Preact

Question

I'm using Preact (for all intents and purposes, React) to render a list of items, saved in a state array. Each item has a remove button next to it. My problem is: when the button is clicked, the proper item is removed (I verified this several time), but the items are re-rendered with the last item missing, and the removed one still there. My code (simplified):

import {Component} from "preact";
import Package from "./package";

export default class Packages extends Component {
  constructor(props) {
    super(props);
    this.state = {packages: ["a", "b", "c", "d", "e"]};
  }

  removePackage(tracking) {
    this.setState(state => (
      {packages: state.packages.filter(item => item !== tracking)}
    ));
  }

  render() {
    const packages = this.state.packages.map((tracking, index) => (
      <div className="package" key={index}>
        <button onClick={() => this.removePackage(tracking)}>
          X
        </button>
        <Package tracking={tracking} />
      </div>
    ));
    return(
      <div>
        <div className="title">Packages</div>
        <div className="packages">{packages}</div>
      </div>
    );
  }
}

What am I doing wrong? Do I need to actively re-render somehow? Is this an n+1 case somehow?

Clarification: My problem is not with the synchronicity of state. In the list above, if I elect to remove "c", the state is updated correctly to ["a", "b", "d", "e"], but the components rendered are ["a", "b", "c", "d"]. At every call to removePackage the correct one is removed from the array, the proper state is shown, but a wrong list is rendered. (I removed the console.log statements, so it won't seem like they are my problem).

Fatigue

Answer 1

This is a classic issue that is totally underserved by Preact's documentation, so I'd like to personally apologize for that! We're always looking for help writing better documentation if anyone is interested.

What has happened here is that you're using the index of your Array as a key (in your map within render). This actually just emulates how a VDOM diff works by default - the keys are always 0-n where n is the array length, so removing any item simply drops the last key off the list.

Explanation: Keys transcend renders

In your example, imagine how the (Virtual) DOM will look on the initial render, and then after removing item "b" (index 3). Below, let's pretend your list is only 3 items long (['a', 'b', 'c']):

Here's what the initial render produces:

<div>
  <div className="title">Packages</div>
  <div className="packages">
    <div className="package" key={0}>
      <button>X</button>
      <Package tracking="a" />
    </div>
    <div className="package" key={1}>
      <button>X</button>
      <Package tracking="b" />
    </div>
    <div className="package" key={2}>
      <button>X</button>
      <Package tracking="c" />
    </div>
  </div>
</div>

Now when we click "X" on the second item in the list, "b" is passed to removePackage(), which sets state.packages to ['a', 'c']. That triggers our render, which produces the following (Virtual) DOM:

<div>
  <div className="title">Packages</div>
  <div className="packages">
    <div className="package" key={0}>
      <button>X</button>
      <Package tracking="a" />
    </div>
    <div className="package" key={1}>
      <button>X</button>
      <Package tracking="c" />
    </div>
  </div>
</div>

Since the VDOM library only knows about the new structure you give it on each render (not how to change from the old structure to the new), what the keys have done is basically tell it that items 0 and 1 remained in-place - we know this is incorrect, because we wanted the item at index 1 to be removed.

Remember: key takes precedence over the default child diff reordering semantics. In this example, because key is always just the 0-based array index, the last item (key=2) just gets dropped off because it's the one missing from the subsequent render.

The Fix

So, to fix your example - you should use something that identifies the item rather than its offset as your key. This can be the item itself (any value is acceptable as a key), or an .id property (preferred because it avoids scattering object references around which can prevent GC):

let packages = this.state.packages.map((tracking, i) => {
  return (
                                  // ↙️ a better key fixes it :)
    <div className="package" key={tracking}>
      <button onClick={this.removePackage.bind(this, tracking)}>X</button>
      <Package tracking={tracking} />
    </div>
  );
});

---

Whew, that was a lot more long-winded that I had intended it to be.

TL,DR: never use an array index (iteration index) as key. At best it's mimicking the default behavior (top-down child reordering), but more often it just pushes all diffing onto the last child.

---

edit: @tommy recommended this excellent link to the eslint-plugin-react docs, which does a better job explaining it than I did above.

Yoruba

Thanks! That was the answer. Man, I tried so many iterations of setState and event handlers, and never even looked at the key :) – Fatigue

Does it leads to this kind of rendering problems only in Preact? I never had these problems on React. – Kumler

Nope, it would actually the same in either. – Yoruba

Hi @JasonMiller can you provide some basic example how to simulate incorrectness when using index please... I've tried here gist.github.com/jurosh/5ef25a18ac3f6b21355ca6a4dd719c55 but cannot get wrong behavior and looks like using index as key is safe. Isn't it really just Preact? – Reactant

It is safe, but often incorrect. You will only see the affect when rendering DOM elements with state, such as inputs with user-entered text in them. It has nothing to do with Preact, this is simply how keys work. Index keys enforce an ordered diff, which is generally not the best type of diff. – Yoruba

Could anyone point out why the second code snippet after "X" is pressed is an undesired render? 1 is now assigned to "c", but the render seems fine... – Maes

@Maes - instead of a single B.remove(), the renderer must re-render B to transform it in-place to match the structure and data of C. Without keys or with proper key usage, the second render should result in a single DOM operation. With index keys, it results in N (depending on the complexity of <Package>). – Yoruba

The explanation above applies equally well for React, right? – Pneumatophore

@T.J.Crowder yes, it applies to React and any other VDOM-style implementation that uses keys to track unique items. – Yoruba

Isn't the order at your example should be ['a', 'b'] instead of the ['a', 'c'] since React will remove the last (affected) key(2 index) and so resulting in ['a', 'b'] rendering instead of the other correct result. – Otherworld

@AbdulrahmanAli no - React will attempt to diff/patch the second subtree as if it were the previous third. – Yoruba

@JasonMiller Can you elaborate what you mean by the previous third? you mean the old subtree? – Otherworld

@JasonMiller But in the OP's question clarification section, the changes he describes during rendering from [a, b, c, d, e] to [a, b, c, d] indicate that React will compare the new state to the old list(tree) and render that instead regardless of the state value. – Otherworld

@AbdulrahmanAli No - the letters in those example arrays are representative of the value of the tracking prop as seen by <Package />. This post was from 2017, which means that the author was using Class Components, and likely did not implement componentWillReceiveProps() to "accept" incoming props. As a result, each <Package> component was "stuck" with its initial prop values, making it seem like Preact had not updated the list. It had updated the list, but it did so by passing new props rather than unmounting. – Yoruba

To clarify: what they expected to happen was this: [updateProps(A, { tracking: 'a' }), updateProps(B, { tracking: 'b' }), unmount(C), updateProps(D, { tracking: 'd' }), updateProps(E, { tracking: 'e' })] ... but what actually happens is this: [updateProps(A, { tracking: 'a' }), updateProps(B, { tracking: 'b' }), updateProps(C, { tracking: 'd' }), updateProps(D, { tracking: 'e' }), unmount(E)] – Yoruba