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

missing key in the demonstration code #4

Open
Pomax opened this issue Jul 18, 2016 · 2 comments
Open

missing key in the demonstration code #4

Pomax opened this issue Jul 18, 2016 · 2 comments

Comments

@Pomax
Copy link

Pomax commented Jul 18, 2016

The demonstration code of https://d4.js.org shows the following example code:

const paths = voronoi(samples)
    .polygons()
    .map(sample => (
      <path
        d={`M${sample.join('L')}Z`}
        fill={color(sample.data)}
        stroke={color(sample.data)}
      />
    ));

but this is missing a key for React to do efficient DOM diffing. Without it you'll get both console warnings, and potentially slow operations where two samples being swapped causes React to instead rebuild them instead of just swapping them.

For properly taking advantage of React, you'll want something like this:

const paths = voronoi(samples)
    .polygons()
    .map(sample => (
      <path
        key={sample.id}
        d={`M${sample.join('L')}Z`}
        fill={color(sample.data)}
        stroke={color(sample.data)}
      />
    ));

where sample.id is a unique identifier for a sample.

And note that, while tempting, you can't use map( (sample,idx) => ....key={idx} ) because the sample's position in the list is not uniquely identifying for the sample (that's only unique information for the specific list the samples are currently in), so successive calls with a reordered array would still make React perform far worse than if a proper unique value is used as key.

@Pomax Pomax changed the title missing ref in the demonstration code missing key in the demonstration code Jul 18, 2016
@tbranyen
Copy link

For the particular example you chose isn't the # of elements predetermined? You should never get additions and replacements happening if you were to animate. For instance I used that code with my own virtual dom tool and found it reused all the inner nodes since they matched the previous. Does React not behave the same way?

Here's my code for comparison:

const width = 960;
const height = 500;
const voronoi = d3.voronoi().extent([[-1, -1], [width + 1, height + 1]]);

const color = (d) => {
  const dx = d[0] - width / 2;
  const dy = d[1] - height / 2;

  return d3.lab(100 - (dx * dx + dy * dy) / 5000, dx / 10, dy / 10);
};

const Mesh = () => {
  const sampler = poissonDiscSampler(width, height, 40);
  const samples = [];
  let sample; while (sample = sampler()) samples.push(sample);

  const paths = voronoi(samples)
    .polygons()
    .map((sample, i) => {
      const fill = color(sample.data).toString();

      return diff.html`<path
        d="M${sample.join('L')}Z"
        fill="${fill}"
        stroke="${fill}"
      />`
    });

  return diff.html`<svg width=${width} height=${height}>${paths}</svg>`;
};

const render = () => {
  diff.innerHTML(document.body.querySelector('main'), Mesh());
  requestAnimationFrame(render);
};

render();

@Pomax
Copy link
Author

Pomax commented Jul 20, 2016

In your exact example it's not a true problem, but people take example code and then tweak it to get their own version, so if someone takes your example but throws in one or more calls that reorder the content (say a toggled .reverse() between the .polygon() and .map() functions) then their code may suddenly behave quite poorly compared to what it could be doing.

Generally it's a good idea to have key attributes even if you're sure the order never changes, because current you can never know for sure whether future you won't change their mind =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants