[Theming] Overhaul Icon Generation

[michaeljaltamirano]

This PR does not require urgent review. The diff is large primarily because we're committing more build artifacts in order to better track changes and modifications.

Background

radiance-ui currently leverages the CLI provided by SVGR to generate all of our icons (as React components) at build time, placed in radiance-ui/lib/icons, using raw svg files and a custom transform template, with the output looking like this:

import React from 'react';
import { css } from '@emotion/core';
import { propTypes, defaultProps, iconStyles } from '../utils/icons';

const SvgAcneBodySprayGlyph = (props) => {
  return (
    <svg
      viewBox="0 0 48 48"
      fill="none"
      css={css`
        ${iconStyles(props)};
      `}
      {...props}
    >
      <path
        d="M23.51 16.32h.57c.72 0 1.31-.59 1.31-1.31v-.52c0-.72-.59-1.31-1.31-1.31h-.57v-.58c0-1.16-1.61-1.69-3.11-1.69-1.5 0-3.11.53-3.11 1.69l.04 3.6c-1.35.43-2.33 1.17-2.33 2.22l.09 18.96c.08 1.36 2.34 2.38 5.25 2.38s5.17-1.02 5.25-2.38v-.02l.08-18.94c0-.95-.96-1.66-2.16-2.1zm0-1.86h.64v.61h-.64v-.61zm-3.1-2.3c1.01 0 1.64.28 1.82.44-.19.16-.82.44-1.82.44s-1.64-.28-1.83-.44c.19-.16.82-.44 1.83-.44zm1.81 5.02v.04c-.17.16-.84.43-1.82.43-.98 0-1.64-.27-1.82-.43v-3.23c.54.2 1.19.3 1.82.3.63 0 1.28-.1 1.82-.3v3.19zm2.14 19.71l-.03.42c-.02.28-1.3 1.2-4 1.2s-3.98-.92-4-1.23l-.02-.29V20.22c1.03.56 2.53.85 4.03.85 1.47 0 2.99-.3 4.03-.87v16.69h-.01zm-4.03-17.07c-2.45 0-4.08-.84-4.08-1.39 0-.28.41-.62 1.12-.9.01.02.01.05.02.07.23.85 1.62 1.3 3.01 1.3 1.39 0 2.79-.45 3.02-1.31v-.01c.62.27.99.58.99.84.01.56-1.62 1.4-4.08 1.4zM28.68 17.96l-.45.43 1.93 2.03.45-.43.46-.43-1.94-2.03-.45.43zM28.97 11.84L31 9.91 30.14 9l-2.03 1.93.43.46.43.45zM29.527 14.02l-.031 1.25 4.468.111.032-1.25-4.47-.111z"
        fill="currentColor"
      />
    </svg>
  );
};

SvgAcneBodySprayGlyph.propTypes = propTypes;
SvgAcneBodySprayGlyph.defaultProps = defaultProps;
export default SvgAcneBodySprayGlyph;

This setup has worked great for us--adding new icons has been as simple as dropping new SVGs in a folder and letting our build setup handle generating the React component and exporting it from the appropriate directory.

However, this setup does not work with our new theming requirements, where the "themeable" icons may be wholesale swapped out for a distinctly different SVGs. By way of visual example, the Arrow and ChevronIcon examples in the screenshots below show totally different icons, made possible by the configuration in this PR.

In order to support this behavior (with the bonus of adding type-safety) we're removing the scaffolding around the SVGR CLI in favor of using the built-in defaults. The CLI requires one file at a time for its transform, so we cannot pass it two files and conditionally return the SVG we want based on props. The setup in this PR prefers straightforwardness, even if it is a bit more manual.

Alternatives

Alternatives to the work in this PR included:

1. Extending our babel reliance.
   • We build our lib folder using babel directly, but there doesn't currently exist an @svgr plugin for this. Airbnb has a plugin that transforms svg imports to directly in-lined code, but it felt incorrect to use a separate babel plugin when we rely on @svgr for rollup and webpack compatibility.
2. Advanced SVG usage
   • I looked into how to conditionally render SVGs. I found material on SVG stacks and nested SVGs, which, in conjunction with a complex @svgr/cli transform template, means we could slip in useTheme logic. However, that would seem to increase the maintenance burden for all parts touched (svg markup, @svgr usage, etc.) to support separate themes within a single SVG file.

This PR does all the manual work of re-configuring the setup with the aim of keeping our setup of adding new SVGs relatively simple, and making sure it is not error-prone.

Adding a new SVG

There are two new considerations for when adding an SVG.

1. Is the SVG new and without a themed counterpart? If so, add the .svg file in the appropriate directory in src/svgs, generate the component icon (via yarn run build:icons:all), and then create a complementary TypeScript file that imports it directly and passes it into useIcon.

// src/icons/icons/arrowLeftIcon.ts
import { ArrowLeftIcon } from './svgs';
import { useIcon, IconProps } from '../../shared-components/icon';

export default (props: IconProps) =>
  // We pass in ArrowLeftIcon to the Secondary arg because there is no Secondary complement (yet). 
  useIcon(ArrowLeftIcon, ArrowLeftIcon, props);

2. Is the SVG a themed counterpart to an existing SVG? If so, add the.svg as a sibling file in src/svgs, and append the icon filenames with -primary and -secondary, respectively. Run the same build step and update the exported icon file (with theming logic) to import both the Primary and Secondary icons:

// This naming is handled automatically by `@svgr`
import { LogoPrimary, LogoSecondary } from './svgs';
import { IconProps, useIcon } from '../../shared-components/icon';

export default (props: IconProps) => useIcon(LogoPrimary, LogoSecondary, props);

yarn run v1.22.5
$ jest src/icons/test.ts
 PASS  src/icons/test.ts
  icons
    exported icons
      logos
        ✓ should have the same number of svg-generated logos pairings, with complementary naming
    generated icons
      logos
        ✓ should have the same number of logos svgs as svg-generated components, with complementary naming (1 ms)

Ideally, committing more of these built files gives us a better understanding of how this repo works. I think it would be nice to include more automatic handling, but as it is I've invested a bit too much time into this, and I don't know how important optimizing our icon build workflow actually is, since they do not change very often.

CHANGELOG

1. src/ and stories/ icon imports updated to use React components with conditional rendering logic, so that when we switch the theme context the icons reflect the changes. (This was not possible with direct .svg imports.)
2. Removes src/utils/icons and src/utils/svgToIconTemplate in favor of src/shared-components/icon and SVGR config options.
3. Removes src/svgs/deprecated (none were in use).
4. Updates SVGR packages
   • The blocker to our updating was due to our heavy reliance on the CLI, which is no longer the case with thees changes.

TODOs

• Update create-index: PR is ready to be merged in following approval of this PR: Add TypeScript/index.ts compatibility create-index#1
• Update storybook
• Update documentation (Will do after PR approval, before merging in--until approval I'm not sure what will need to be documented and what will change)
• Double-check the snapshot diff is O.K. by digging more into Icon/useIcon behavior.
  • We're actually capturing more detail now!

[michaeljaltamirano]

The index.tsx file generated by @svgr/cli lacks a newline. Without this, the respective files will always appear to have been modified, which is annoying. 🙃

[michaeljaltamirano]

Fixes build warning.

[michaeljaltamirano]

The application of emotion-4 on the svg in the snapshot indicates there's no regression with the behavior we were achieving with the custom @svgr template, that put the css prop directly on the <svg> element.

[michaeljaltamirano]

This shakes up the dist build artifact since the icons included in Component logic now reside in dist/bundle-es/icons/icons instead of dist/bundle-es/svgs, so the file ordering got jumbled around and it's hard to track changes. It's always nice to see a smaller bundle, though 🥳

[michaeljaltamirano]

The default behavior of create-index is to generate an index.ts file comprising default exports renamed as their "safe" named export value. It automatically picks up that this should be export { default as AnxiousEmoji } from the filename.

There's good React reasons to not use anonymous default exports--the component shows up as Anonymous in the Component tree in dev tools--but since the underlying components in useIcon will be named (by @svgr/webpack), this seemed a fine trade-off to make for maintaining compatibility with create-index.

Moreover, anonymous default exports reduces auto-import name-spacing noise a bit. (Currently if you add AnxiousEmoji in a file, the auto-import will suggests both ../../src/icons and ../../src/icons/emojis as relative imports, and TypeScript definitions work best with relative imports.)

[daigof]

Looks good, I like this solution, I like how you got rid of all those confusing ast templates.

Left 1 comment regarding @svgr/webpack and I guess I have more cleanups comments for future PRs:

• we have 3 babel configs: I guess we can remove the babel.config at root since it was used by svgr/webpack the other 2 are for build and storybook, wonder if we can combine those somehow
• With this PR I think we reached 100% TS coversion. -> we can remove rules extensions that have js in many configs I also saw a md somewhere

[michaeljaltamirano]

Looks good, I like this solution, I like how you got rid of all those confusing ast templates.

Left 1 comment regarding @svgr/webpack and I guess I have more cleanups comments for future PRs:

• we have 3 babel configs: I guess we can remove the babel.config at root since it was used by svgr/webpack the other 2 are for build and storybook, wonder if we can combine those somehow
• With this PR I think we reached 100% TS coversion. -> we can remove rules extensions that have js in many configs I also saw a md somewhere

Simplified the storybook config and removed the @svgr/webpack dependency. I have a follow-up PR planned for streamlining the babel conifg.