[Icons] Transpile icon SVGs with display name #9871

fortmarek · 2023-08-02T11:52:47Z

WHY are these changes introduced?

For full context, read here.

We are introducing a displayName prop to be able to dynamically read the icon name when we have access only to the Polaris icon component function:

import {AbandondedCartMajor} from '@shopify/polaris-icons';

const iconName = AbandondedCartMajor.displayName;
/* Do something with displayName */

// This wouldn't work as the `name` gets minified
// const iconName = AbandonedCartMajor.name;

React components generally use the convention of displayName already and we use this convention in Polaris as well.

WHAT is this pull request doing?

For transpiling the SVG files, we use the svgr library. Ideally, this library would by default add the displayName but this was rejected by the author.

Instead, we can provide our own template to be used by the svgr library.
The only difference from the default template is the following file:

${componentName}.displayName = "${componentName.name.replace(/^Svg/g, '')}";

You can check the before and after of dist/icons/AbandondedCartMajor.svg.js here:

Before

'use strict';

var React = require('react');

var SvgAbandonedCartMajor = function SvgAbandonedCartMajor(props) {
  return /*#__PURE__*/React.createElement("svg", Object.assign({
    viewBox: "0 0 20 20"
  }, props), /*#__PURE__*/React.createElement("path", {
    d: "M3.25 3a.75.75 0 0 0 0 1.5h1.612a.25.25 0 0 1 .248.22l1.04 8.737a1.75 1.75 0 0 0 1.738 1.543h6.362a.75.75 0 0 0 0-1.5h-6.362a.25.25 0 0 1-.248-.22l-.093-.78h6.35a2.75 2.75 0 0 0 2.743-2.54l.358-4.652a.75.75 0 0 0-1.496-.116l-.358 4.654a1.25 1.25 0 0 1-1.246 1.154h-6.53l-.768-6.457a1.75 1.75 0 0 0-1.738-1.543h-1.612Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M8.87 5.12a.75.75 0 0 0 0 1.06l1.32 1.32-1.32 1.32a.75.75 0 1 0 1.06 1.06l1.32-1.32 1.32 1.32a.75.75 0 0 0 1.06-1.06l-1.32-1.32 1.32-1.32a.75.75 0 0 0-1.06-1.06l-1.32 1.32-1.32-1.32a.75.75 0 0 0-1.06 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M10 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M15 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }));
};

exports.SvgAbandonedCartMajor = SvgAbandonedCartMajor;

After

'use strict';

var React = require('react');

var SvgAbandonedCartMajor = function SvgAbandonedCartMajor(props) {
  return /*#__PURE__*/React.createElement("svg", Object.assign({
    viewBox: "0 0 20 20"
  }, props), /*#__PURE__*/React.createElement("path", {
    d: "M3.25 3a.75.75 0 0 0 0 1.5h1.612a.25.25 0 0 1 .248.22l1.04 8.737a1.75 1.75 0 0 0 1.738 1.543h6.362a.75.75 0 0 0 0-1.5h-6.362a.25.25 0 0 1-.248-.22l-.093-.78h6.35a2.75 2.75 0 0 0 2.743-2.54l.358-4.652a.75.75 0 0 0-1.496-.116l-.358 4.654a1.25 1.25 0 0 1-1.246 1.154h-6.53l-.768-6.457a1.75 1.75 0 0 0-1.738-1.543h-1.612Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M8.87 5.12a.75.75 0 0 0 0 1.06l1.32 1.32-1.32 1.32a.75.75 0 1 0 1.06 1.06l1.32-1.32 1.32 1.32a.75.75 0 0 0 1.06-1.06l-1.32-1.32 1.32-1.32a.75.75 0 0 0-1.06-1.06l-1.32 1.32-1.32-1.32a.75.75 0 0 0-1.06 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M10 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M15 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }));
};
SvgAbandonedCartMajor.displayName = "AbandonedCartMajor";

exports.SvgAbandonedCartMajor = SvgAbandonedCartMajor;

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Add the following line to Icon.stories.tsx:

console.log(CirclePlusMinor.displayName);

When you open the story, the displayName should be printed.

🎩 checklist

Tested on mobile
Tested on multiple browsers
~~Tested for accessibility~~
~~Updated the component's README.md with documentation changes~~
~~Tophatted documentation changes in the style guide~~

.changeset/famous-rats-invent.md

kyledurand · 2023-08-02T14:20:06Z

polaris-icons/icon-template.js

+    plugins.push('typescript');
+  }
+  const typeScriptTpl = template.smart({plugins});
+  return typeScriptTpl.ast`${imports}


I don't have context on these kinds of things but this file seems confusing to me. With this unguarded return here is anything below even getting run?

We use the default template, so I don't want to change it more than we have to.

"I don't want to change it more than we have to." - Every Design System Engineer ever.
Wise words. +1000
@fortmarek 😂

kyledurand · 2023-08-02T15:57:42Z

/snapit

github-actions · 2023-08-02T16:03:18Z

🫰✨ Thanks @kyledurand! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230802160056

yarn add @shopify/polaris-icons@0.0.0-snapshot-release-20230802160056

yarn add @shopify/polaris@0.0.0-snapshot-release-20230802160056

kyledurand · 2023-08-02T16:10:02Z

Something to keep in mind with this is that it will increase the bundle size. Are you absolutely sure you need to add this?

sam-b-rose · 2023-08-02T19:25:10Z

this is that it will increase the bundle size.

@kyledurand I think we are removing the displayName for production builds in web? If consumers have that in their bundler, I think we shouldn't be too concerned.

ssetem · 2023-08-02T19:57:26Z

this is that it will increase the bundle size.

@kyledurand I think we are removing the displayName for production builds in web? If consumers have that in their bundler, I think we shouldn't be too concerned.

~~Hmm, we definitely will need this in production also, have you got more details on the displayName removal?~~

Edit I think this will stay in production since we are using displayName as a static variable

kyledurand · 2023-08-02T20:07:44Z

I don't see how they'd be removed in prod. They get bundled as react components

So this:

'use strict';

var React = require('react');

var SvgAbandonedCartMajor = function SvgAbandonedCartMajor(props) {
  return /*#__PURE__*/React.createElement("svg", Object.assign({
    viewBox: "0 0 20 20"
  }, props), /*#__PURE__*/React.createElement("path", {
    d: "M3.25 3a.75.75 0 0 0 0 1.5h1.612a.25.25 0 0 1 .248.22l1.04 8.737a1.75 1.75 0 0 0 1.738 1.543h6.362a.75.75 0 0 0 0-1.5h-6.362a.25.25 0 0 1-.248-.22l-.093-.78h6.35a2.75 2.75 0 0 0 2.743-2.54l.358-4.652a.75.75 0 0 0-1.496-.116l-.358 4.654a1.25 1.25 0 0 1-1.246 1.154h-6.53l-.768-6.457a1.75 1.75 0 0 0-1.738-1.543h-1.612Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M8.87 5.12a.75.75 0 0 0 0 1.06l1.32 1.32-1.32 1.32a.75.75 0 1 0 1.06 1.06l1.32-1.32 1.32 1.32a.75.75 0 0 0 1.06-1.06l-1.32-1.32 1.32-1.32a.75.75 0 0 0-1.06-1.06l-1.32 1.32-1.32-1.32a.75.75 0 0 0-1.06 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M10 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M15 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }));
};

exports.SvgAbandonedCartMajor = SvgAbandonedCartMajor;

Becomes this

'use strict';

var React = require('react');

var SvgAbandonedCartMajor = function SvgAbandonedCartMajor(props) {
  return /*#__PURE__*/React.createElement("svg", Object.assign({
    viewBox: "0 0 20 20"
  }, props), /*#__PURE__*/React.createElement("path", {
    d: "M3.25 3a.75.75 0 0 0 0 1.5h1.612a.25.25 0 0 1 .248.22l1.04 8.737a1.75 1.75 0 0 0 1.738 1.543h6.362a.75.75 0 0 0 0-1.5h-6.362a.25.25 0 0 1-.248-.22l-.093-.78h6.35a2.75 2.75 0 0 0 2.743-2.54l.358-4.652a.75.75 0 0 0-1.496-.116l-.358 4.654a1.25 1.25 0 0 1-1.246 1.154h-6.53l-.768-6.457a1.75 1.75 0 0 0-1.738-1.543h-1.612Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M8.87 5.12a.75.75 0 0 0 0 1.06l1.32 1.32-1.32 1.32a.75.75 0 1 0 1.06 1.06l1.32-1.32 1.32 1.32a.75.75 0 0 0 1.06-1.06l-1.32-1.32 1.32-1.32a.75.75 0 0 0-1.06-1.06l-1.32 1.32-1.32-1.32a.75.75 0 0 0-1.06 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M10 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }), /*#__PURE__*/React.createElement("path", {
    d: "M15 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
  }));
};
SvgAbandonedCartMajor.displayName = "AbandonedCartMajor";

exports.SvgAbandonedCartMajor = SvgAbandonedCartMajor;

kyledurand

I'm fine with increasing the bundle size if this is necessary. It's definitely a nice to have anyway

fortmarek · 2023-08-03T07:25:57Z

Thanks for the reviews! The increase in bundle size (3 %) is unfortunate but we highly need this feature.

@ardakaracizmeli

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris-icons@7.8.0 ### Minor Changes - [#9858](#9858) [`3fc3d5923`](3fc3d59) Thanks [@ardakaracizmeli](https://github.com/ardakaracizmeli)! - Added SidekickMajor - [#9871](#9871) [`bac86a621`](bac86a6) Thanks [@fortmarek](https://github.com/fortmarek)! - Added displayName to transpiled icons - [#9854](#9854) [`5dabf0fe0`](5dabf0f) Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Updated CancelMajor and CancelMinor icons ## @shopify/polaris@11.11.0 ### Minor Changes - [#9697](#9697) [`c078d5d85`](c078d5d) Thanks [@nat-king](https://github.com/nat-king)! - Added optional prop `TextDecorationLine` to `Text` to include a line-through decoration ### Patch Changes - [#9847](#9847) [`85b68a358`](85b68a3) Thanks [@qt314](https://github.com/qt314)! - Added `role` prop to `VerticalStack` - [#9863](#9863) [`4061fd04d`](4061fd0) Thanks [@zakwarsame](https://github.com/zakwarsame)! - Fixed ActionList item overflow and tooltip zIndex - Updated dependencies \[[`3fc3d5923`](3fc3d59), [`bac86a621`](bac86a6), [`5dabf0fe0`](5dabf0f)]: - @shopify/polaris-icons@7.8.0 ## @shopify/polaris-cli@0.2.29 ## polaris.shopify.com@0.56.7 ### Patch Changes - Updated dependencies \[[`3fc3d5923`](3fc3d59), [`85b68a358`](85b68a3), [`bac86a621`](bac86a6), [`4061fd04d`](4061fd0), [`5dabf0fe0`](5dabf0f), [`c078d5d85`](c078d5d)]: - @shopify/polaris-icons@7.8.0 - @shopify/polaris@11.11.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

### WHY are these changes introduced? For full context, read [here](https://github.com/Shopify/web/pull/100163). We are introducing a `displayName` prop to be able to dynamically read the icon name when we have access only to the Polaris icon component function: ```tsx import {AbandondedCartMajor} from '@shopify/polaris-icons'; const iconName = AbandondedCartMajor.displayName; /* Do something with displayName */ // This wouldn't work as the `name` gets minified // const iconName = AbandonedCartMajor.name; ``` React components generally use the convention of `displayName` already and we use this convention in [Polaris as well](https://github.com/search?q=repo%3AShopify%2Fpolaris%20displayname&type=code).  ### WHAT is this pull request doing? For transpiling the SVG files, we use the [svgr](https://github.com/gregberge/svgr) library. Ideally, this library would by default add the `displayName` but this was [rejected by the author](gregberge/svgr#328). Instead, we can provide our [own template](https://react-svgr.com/docs/migrate/#template) to be used by the `svgr` library. The only difference from the default template is the following file: ``` ${componentName}.displayName = "${componentName.name.replace(/^Svg/g, '')}"; ``` You can check the before and after of `dist/icons/AbandondedCartMajor.svg.js` here: <details> <summary>Before</summary> ```tsx 'use strict'; var React = require('react'); var SvgAbandonedCartMajor = function SvgAbandonedCartMajor(props) { return /*#__PURE__*/React.createElement("svg", Object.assign({ viewBox: "0 0 20 20" }, props), /*#__PURE__*/React.createElement("path", { d: "M3.25 3a.75.75 0 0 0 0 1.5h1.612a.25.25 0 0 1 .248.22l1.04 8.737a1.75 1.75 0 0 0 1.738 1.543h6.362a.75.75 0 0 0 0-1.5h-6.362a.25.25 0 0 1-.248-.22l-.093-.78h6.35a2.75 2.75 0 0 0 2.743-2.54l.358-4.652a.75.75 0 0 0-1.496-.116l-.358 4.654a1.25 1.25 0 0 1-1.246 1.154h-6.53l-.768-6.457a1.75 1.75 0 0 0-1.738-1.543h-1.612Z" }), /*#__PURE__*/React.createElement("path", { d: "M8.87 5.12a.75.75 0 0 0 0 1.06l1.32 1.32-1.32 1.32a.75.75 0 1 0 1.06 1.06l1.32-1.32 1.32 1.32a.75.75 0 0 0 1.06-1.06l-1.32-1.32 1.32-1.32a.75.75 0 0 0-1.06-1.06l-1.32 1.32-1.32-1.32a.75.75 0 0 0-1.06 0Z" }), /*#__PURE__*/React.createElement("path", { d: "M10 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z" }), /*#__PURE__*/React.createElement("path", { d: "M15 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z" })); }; exports.SvgAbandonedCartMajor = SvgAbandonedCartMajor; ``` </details> <details> <summary>After</summary> ```tsx 'use strict'; var React = require('react'); var SvgAbandonedCartMajor = function SvgAbandonedCartMajor(props) { return /*#__PURE__*/React.createElement("svg", Object.assign({ viewBox: "0 0 20 20" }, props), /*#__PURE__*/React.createElement("path", { d: "M3.25 3a.75.75 0 0 0 0 1.5h1.612a.25.25 0 0 1 .248.22l1.04 8.737a1.75 1.75 0 0 0 1.738 1.543h6.362a.75.75 0 0 0 0-1.5h-6.362a.25.25 0 0 1-.248-.22l-.093-.78h6.35a2.75 2.75 0 0 0 2.743-2.54l.358-4.652a.75.75 0 0 0-1.496-.116l-.358 4.654a1.25 1.25 0 0 1-1.246 1.154h-6.53l-.768-6.457a1.75 1.75 0 0 0-1.738-1.543h-1.612Z" }), /*#__PURE__*/React.createElement("path", { d: "M8.87 5.12a.75.75 0 0 0 0 1.06l1.32 1.32-1.32 1.32a.75.75 0 1 0 1.06 1.06l1.32-1.32 1.32 1.32a.75.75 0 0 0 1.06-1.06l-1.32-1.32 1.32-1.32a.75.75 0 0 0-1.06-1.06l-1.32 1.32-1.32-1.32a.75.75 0 0 0-1.06 0Z" }), /*#__PURE__*/React.createElement("path", { d: "M10 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z" }), /*#__PURE__*/React.createElement("path", { d: "M15 17a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z" })); }; SvgAbandonedCartMajor.displayName = "AbandonedCartMajor"; exports.SvgAbandonedCartMajor = SvgAbandonedCartMajor; ``` </details>   ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)  Add the following line to `Icon.stories.tsx`: ``` console.log(CirclePlusMinor.displayName); ``` When you open the story, the `displayName` should be printed. ### 🎩 checklist - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [x] ~Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)~ - [x] ~Updated the component's `README.md` with documentation changes~ - [x] ~[Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide~

fortmarek changed the title ~~Transpile icon SVGs with display name~~ (@shopify/polaris-icons): Transpile icon SVGs with display name Aug 2, 2023

fortmarek force-pushed the polaris-icons/display-name branch from 5d12732 to 4a119d9 Compare August 2, 2023 12:37

fortmarek requested review from ssetem and sam-b-rose August 2, 2023 12:37

fortmarek marked this pull request as ready for review August 2, 2023 12:37

ssetem requested a review from kyledurand August 2, 2023 13:57

kyledurand reviewed Aug 2, 2023

View reviewed changes

fortmarek force-pushed the polaris-icons/display-name branch from 4a119d9 to 776d031 Compare August 2, 2023 15:18

Transpile icon SVGs with display name

48fbd87

fortmarek force-pushed the polaris-icons/display-name branch from 776d031 to 48fbd87 Compare August 2, 2023 15:49

sam-b-rose approved these changes Aug 2, 2023

View reviewed changes

kyledurand approved these changes Aug 2, 2023

View reviewed changes

fortmarek changed the title ~~(@shopify/polaris-icons): Transpile icon SVGs with display name~~ [Icons] Transpile icon SVGs with display name Aug 3, 2023

fortmarek merged commit bac86a6 into main Aug 3, 2023

fortmarek deleted the polaris-icons/display-name branch August 3, 2023 07:27

shopify-github-actions-access bot mentioned this pull request Aug 3, 2023

Version Packages #9855

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Icons] Transpile icon SVGs with display name #9871

[Icons] Transpile icon SVGs with display name #9871

fortmarek commented Aug 2, 2023 •

edited by ssetem

Loading

kyledurand Aug 2, 2023

fortmarek Aug 2, 2023 •

edited

Loading

jasperfurniss Aug 21, 2023

kyledurand commented Aug 2, 2023

github-actions bot commented Aug 2, 2023

kyledurand commented Aug 2, 2023

sam-b-rose commented Aug 2, 2023

ssetem commented Aug 2, 2023 •

edited

Loading

kyledurand commented Aug 2, 2023

kyledurand left a comment

fortmarek commented Aug 3, 2023

[Icons] Transpile icon SVGs with display name #9871

[Icons] Transpile icon SVGs with display name #9871

Conversation

fortmarek commented Aug 2, 2023 • edited by ssetem Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to 🎩

🎩 checklist

kyledurand Aug 2, 2023

Choose a reason for hiding this comment

fortmarek Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

jasperfurniss Aug 21, 2023

Choose a reason for hiding this comment

kyledurand commented Aug 2, 2023

github-actions bot commented Aug 2, 2023

kyledurand commented Aug 2, 2023

sam-b-rose commented Aug 2, 2023

ssetem commented Aug 2, 2023 • edited Loading

kyledurand commented Aug 2, 2023

kyledurand left a comment

Choose a reason for hiding this comment

fortmarek commented Aug 3, 2023

fortmarek commented Aug 2, 2023 •

edited by ssetem

Loading

fortmarek Aug 2, 2023 •

edited

Loading

ssetem commented Aug 2, 2023 •

edited

Loading