fix(Icon): Support null tagName in Icon component

[ggdouglas]

Fixes #6974

Checklist

• Includes tests
• Update documentation

Changes proposed in this pull request:

Fixes a condition where a null element gets passed into React.createElement while the icon is loading paths asynchronously. This prevents an error in the Icon component when null is passed in as the tagName.

[changelog-app]

Generate changelog in packages/core/changelog/@unreleased

What do the change types mean?

• feature: A new feature of the service.
• improvement: An incremental improvement in the functionality or operation of the service.
• fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
• break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
  and external consumers of the service's API (e.g. customer-written software or integrations).
• deprecation: Advertises the intention to remove service functionality without any change to the
  operation of the service itself.
• manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
  performing database surgery, ...) at the time of upgrade for it to succeed.
• migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

• ❗The break and manual task changelog types will result in a major release!
• 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
• ✨ All others will result in a minor version release.

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

fix(Icon): Support null tagName in Icon component

Check the box to generate changelog(s)

• Generate changelog entry

[svc-palantir-github]

Update test to check for presence of span element

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[ggdouglas]

I think it makes sense to ensure this value is always a valid element. The alternative would be to do something like a null check and return early, e.g.

if (tagName == null) {
    return null;
}
return React.createElement(tagName, { ... });

The former will show a blip of the fallback while the icon paths load:

whereas the later will appear and shift in without any placeholder at all:

[jscheiny]

Any particular reason to use || here over ???

[ggdouglas]

Hm, I suppose the only main difference is that || will use "span" as the default for any falsy value, including "", 0, and false. If the user is consuming this component in TypeScript, there is functionally no difference since tagName is either type ofReact.JSX.IntrinsicElements or null.

Do we care about this edge case for non-TypeScript consumers?

[jscheiny]

Yeah okay, that makes sense. I think its fair to keep this for non-TS consumers.