Noumenon72 9 days ago

I thought the example comment should strive to be shorter than this:

    // Currently, key can be spread in as a prop. This causes a potential
    // issue if key is also explicitly declared (ie. <div {...props} key="Hi" />
    // or <div key="Hi" {...props} /> ). We want to deprecate key spread,
    // but as an intermediary step, we will use jsxDEV for everything except
    // <div {...props} key="Hi" />, because we aren't currently able to tell if
    // key is explicitly declared to be undefined or not.
    if (maybeKey !== undefined) {
      key = '' + maybeKey
    }
But I found while trying to shorten it that a) it couldn't be that much shorter and b) it actually contained all the info I needed to understand it, which is the goal. Rewriting to my preference would just be my way of understanding it.

Anyway, here's what I would write to explain the why:

    // If the JSX key was passed explicitly, use it. This may overwrite a `key`
    // from spread props, e.g. <div {...props} key="Hi" />. We can't distinguish
    // that from <div key="Hi" {...props} />, so until key spreading is deprecated,
    // we ask people to use `jsxDEV` instead of `jsx` for all cases except
    // <div {...props} key="Hi" />.
This explains:

* Why the line of code is here: to ensure explicitly passed `key` wins. The 'what' is "coerce maybeKey to a string", which doesn't clearly address the issue of key spread

* State the issue with _this line of code_, which is that it overwrites user-specified keys in props, not just that it's "a potential issue".

* State which example is good and which is bad and requires jsxDEV -- actionable advice.

I also think the `key` param should be documented to say

    This will overwrite `key` in spread props. Use `jsxDEV` if you want to do <div key="Hi" {...props} />.
Otherwise, you've only explained the issue to internal developers, while framework users get no warning until they're so far into debugging they've narrowed it down to this function.