nested loops - must specify keys

[KittenHero]

example:

props => pug`
for row, y in props.grid
  for cell, x in row
    span(key=y*row.length + x)  #{cell}
`

workaround - use a flat array or insert a grouping element

props => pug`
for row, y in props.grid
  div(key=y)
    for cell, x in row
      span(key=x) #{cell}
`

[ForbesLindesay]

It's not clear from this exactly what the problem is, or what you would like to be changed in babel-plugin-transform-react-pug

[KittenHero]

well, I don't think that it makes sense to ask for a key if there's no top level element being emitted in the for loop body

[ForbesLindesay]

I'd accept a pull request to that effect. The challenge is making sure that

for x in things
  if x
    div= x

still requires a key for the div.

[KittenHero]

I'm not familiar with the code base but i'll see what I can do

[ezhlobo]

@ForbesLindesay hey, I took a look at the related part of the code a bit and found, that we have some important checks internally: like requiring keys for loops and in other cases. But they are not obligatory for react, it can handle everything fine. The problem with keys is that they are necessary to keep reconcilation (read as handling re-render) correct. So we might not need excessive checks on our end, right?

Without stopping transpiling on keys, people will get notification from react in development mode anyway. So I believe we should get rid of such parts of the code. What do you think?

[ForbesLindesay]

We currently use arrays for various internal things, e.g.

const element = pug`
  div
    if Math.random() > 0.5
      p a couple of
      p elements
`;

currently produces an array containing the two paragraphs. We know that these don't really need a key, but React would warn about the errors that occur during reconciliation. Now that React.Fragment exists, this problem could actually be fixed. If we convert all the places where we currently return array literals (i.e. arrays where we already know how many elements there will be) and have them return React.Fragment elements, we can then remove all the automated key generation. This will expose React's built in warning. This would then suggest that code like:

pug`
  for row, y in props.grid
    for cell, x in row
      span(key=y*row.length + x)  a
      span  b
`;

should generate something like:

props.grid.map((row, y) => row.map((cell, x) =>
  <React.Fragment key={y*row.length + x}>
    <span>a</span>
    <span>b</span>
  </React.Fragment>
);

Note that here, we have to automatically move the key from the span onto the containing React.Fragment. Does this all make sense? I may have made mistakes.

[ezhlobo]

@ForbesLindesay @KittenHero so, my intention here is to let pug not care about keys at all. I don't see any reason to validate the code before transpiling, because even React throws warnings instead of errors (like we do).

BTW, if we let people transpile the code without keys, they will still see the warnings from React in DEV mode.

If you strongly disagree, let me know.

[ezhlobo]

Done in v7.0.0

See how it works online

//CC @KittenHero