Skip to content

`<Icon />`: Update to support `@mdi/react` instead of `mdi-react`

Warren Giffordrequested to merge
tr/icon-v2 into main

Created by: umpox

Description

This PR:

  • Adds support for @mdi/react and @mdi/js as an preferred alternative to mdi-react.

@mdi/js vs mdi-react

Problem: mdi-react gives us a lot less control over our SVG elements. We are limited in what we can pass down to our Icons (e.g. we can't inject title or description elements that are valuable for accessibility). It also doesn't allow us to access the ref of the element, which is essential for hooking events up to our icons (e.g. for Tooltips)

The new library:

  • Allows injecting elements into our icons (like <title> or <description>)
  • Supports forwardRef, so we can access the <svg> element directly
  • Supports all the same icons
  • Has similar/slightly more usage according to NPM
  • Is actively maintained by the person who also runs materialdesignicons (which both libraries source from)
  • Should be more performant, as we're no longer duplicating the SVG boilerplate through our imports, we now only import the path which is the only unique thing per import.

Migration plan

  1. Update <Icon /> to support both libraries.
  2. Run codemod to migrate simple usage of mdi-react icons to @mdi/react a. Codemod PR: https://github.com/sourcegraph/codemod/pull/140 b. Current migration PRs (to test codemod and migration) https://github.com/sourcegraph/sourcegraph/pull/37411, https://github.com/sourcegraph/sourcegraph/pull/37409, https://github.com/sourcegraph/sourcegraph/pull/37410
  3. Manually update any complex usage.
  4. Remove mdi-react import from application

App preview:

Check out the client app preview documentation to learn more.

Test plan

Ran application locally, and in Storybook. Also tested with a screen reader to ensure accessibility isn't lost with the new component

Merge request reports