fix(factories): enforce generateKey property

[layershifter]

This PR enforces generateKey option to force the proper configuration of factory usages:

• generateKey=true inside collections, i.e. ListItem.create({}, { generateKey: true }) in List
• generateKey=false in all other cases, i.e. Icon.create({}, { generateKey: false }) in MenuItem

As other possible option we can set it to false by default instead of true.

Goals 🥇

Avoid collision in generated keys

See Semantic-Org/Semantic-UI-React#2424 for more details.

Avoid useless remounts

For example content is a string that will be passed to Tooltip component:

<Tooltip content={content} />

The content slot there is represented by TooltipContent component. As our current default value is true, it forces to generate keys always. And this will cause the component's remount on every string change 👎

[DustyTheBot]

	Warnings
⚠️	There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Perf comparision

Scenario	Fluent TPI	Fabric TPI	Ratio	Iterations	Ticks
Button.Fluent	1.34	0.15	8.93:1	5000	6709
Checkbox.Fluent	1.36	0.23	5.91:1	5000	6777
Icon.Fluent	0.25	0.04	6.25:1	5000	1261
Slider.Fluent	1.75	0.25	7:1	5000	8725

Generated by 🚫 dangerJS

[levithomason]

This might be removed?

This option is only needed due to "primitive shorthand collection" values.

• If we remove collection shorthand (feat(List): add proper support for children API #2207), we won't need this option.
• If we remove primitive shorthand values (RFC: drop support for primitive shorthand values #2206), we don't need this option.

If we go with either of these, we can just delete the option and tell consumers to use key when they need to.

Breaking Change?

This could this be a breaking change for consumers, if merged. Their tests could rely on having keys or things like animations which also need keys sometimes even when not in a collection.

Repeating history?

Lastly, I cannot recall why but it seems we used to once use a default of false and it didn't work out in Semantic UI React:

Semantic-Org/Semantic-UI-React#2418

Each of those .create() methods can accept an options object as a second argument that gets passed through to createShorthand() - a flag could be added that disables the autogenerated key

We used to enable generateKey as a createShorthandFactory() option which is not possible as we don't know if/when keys will be needed for a given factory. However, disabling keys at the time of creation seems like a good solution.

I'd gladly merge a PR for this!

[layershifter]

This might be removed?

Yep, if any of this proposals will be merged this will be not needed anymore. I see this PR as a temporary resolution.

Their tests could rely on having keys or things like animations which also need keys sometimes even when not in a collection.

Can you please provide a case? Enzyme allows only to get keys, but do not select as key is not a real prop, enzymejs/enzyme#645. I also don't see there a case for animations as keys will not generated only for slots inside component. And it's valid because React generates keys automatically for JSX markup.

Repeating history?

Yep, that's why Earth is flat 😁 That's why I don't consider false as default value and decided to force its setup.