Wrong components rendered by Preact

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 { h, Component } from 'preact';
import Package from './package';

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

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

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

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).

Solution 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.