Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Icon): correctly parse function with default props #1713

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

arteria32
Copy link
Contributor

Now, we can catch a bag when the size or width and height are not specified directly and transform the input data for the icon by svgr.

**Example of transformed by svgr data svg from gravity-ui/icons/ **

var ArrowUpRightFromSquareIcon = function ArrowUpRightFromSquareIcon(props) {
  return /*#__PURE__*/_jsx("svg", _objectSpread(_objectSpread({}, props), {}, {
    children: /*#__PURE__*/_jsx("path", {
      fill: "currentColor",
      fillRule: "evenodd",
      d: "M10 1.5A.75.75 0 0 0 10 3h1.94L6.97 7.97a.75.75 0 0 0 1.06 1.06L13 4.06V6a.75.75 0 0 0 1.5 0V2.25a.75.75 0 0 0-.75-.75H10ZM7.5 3.25a.75.75 0 0 0-.75-.75H4.5a3 3 0 0 0-3 3v6a3 3 0 0 0 3 3h6a3 3 0 0 0 3-3V9.25a.75.75 0 0 0-1.5 0v2.25a1.5 1.5 0 0 1-1.5 1.5h-6A1.5 1.5 0 0 1 3 11.5v-6A1.5 1.5 0 0 1 4.5 4h2.25a.75.75 0 0 0 .75-.75Z",
      clipRule: "evenodd"
    })
  }));
};
ArrowUpRightFromSquareIcon.defaultProps = {
  xmlns: "http://www.w3.org/2000/svg",
  width: "16",
  height: "16",
  fill: "none",
  viewBox: "0 0 16 16"
};

Creating a component as a usual function skips the default props of the component.
Let's change the method of creating the SVGR component to getting props.


I hereby agree to the terms of the CLA available at: CLA.

@arteria32 arteria32 marked this pull request as ready for review July 18, 2024 11:48
@arteria32 arteria32 requested a review from amje as a code owner July 18, 2024 11:48
@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@amje amje changed the title fix(Icon): Change the method to create the SVG component fix(Icon): сhange the method to create the SVG component Jul 18, 2024
@gravity-ui-bot
Copy link
Contributor

Visual Tests Report is ready.

@amje amje changed the title fix(Icon): сhange the method to create the SVG component fix(Icon): use React.createElement on creating SVG component Jul 18, 2024
@@ -71,7 +71,7 @@ export const Icon: React.ForwardRefExoticComponent<IconProps & React.RefAttribut
} else if (isComponentSvgData(data)) {
({viewBox} = data.defaultProps);
} else if (isSvgrData(data)) {
const el = data({}) as React.ReactElement;
const el = React.createElement(data, {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see in line 125, we don't want the default props to be applied. It was intentional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I understand.
But on line 61, you would like to get the width and height from the viewBox in case they were not specified.
And the viewBox is passed to defaultProps in svgr transformed icons.
sorry, if I miss something

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is data({}) and React.createElement(data, {}) produce different results. Return type of React.createElement(data, {}) is not svg element and it hasn't the viewBox prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I can try a alternative solution- extends available types for component svg data

export function isComponentSvgData(data: SVGIconData): data is SVGIconComponentData {
    return (typeof data === 'object' || typeof data === 'function') && 'defaultProps' in data;
}

@amje
Copy link
Contributor

amje commented Jul 18, 2024

@arteria32 I tried to get same result as yours when transforming via SVGR in the playground but I couldn't. How do you get function component + defaultProps code?

@arteria32
Copy link
Contributor Author

@arteria32 I tried to get same result as yours when transforming via SVGR in the playground but I couldn't. How do you get function component + defaultProps code?

Example:
1 step. Create component
src/..../IconWithoutSize.tsx

export const IconWithoutSize = () => {
    return <Icon data={ArrowUpRightFromSquareIcon} />;
};

2 step: We build project with gravity-ui/app-builder

import {LibraryConfig} from '@gravity-ui/app-builder';

const config: LibraryConfig = {
    lib: {
        internalDirs: ['i18n/keysets'],
    },
};

export default config;

3 step: Get a bad component in build package in build/esm/components/.../IconWithoutSize/IconWithoutSize.js

import _objectSpread from "@babel/runtime/helpers/objectSpread2";
var ArrowUpRightFromSquareIcon = function ArrowUpRightFromSquareIcon(props) {
  return /*#__PURE__*/_jsx("svg", _objectSpread(_objectSpread({}, props), {}, {
    children: /*#__PURE__*/_jsx("path", {
      fill: "currentColor",
      fillRule: "evenodd",
      d: "M10 1.5A.75.75 0 0 0 10 3h1.94L6.97 7.97a.75.75 0 0 0 1.06 1.06L13 4.06V6a.75.75 0 0 0 1.5 0V2.25a.75.75 0 0 0-.75-.75H10ZM7.5 3.25a.75.75 0 0 0-.75-.75H4.5a3 3 0 0 0-3 3v6a3 3 0 0 0 3 3h6a3 3 0 0 0 3-3V9.25a.75.75 0 0 0-1.5 0v2.25a1.5 1.5 0 0 1-1.5 1.5h-6A1.5 1.5 0 0 1 3 11.5v-6A1.5 1.5 0 0 1 4.5 4h2.25a.75.75 0 0 0 .75-.75Z",
      clipRule: "evenodd"
    })
  }));
};
ArrowUpRightFromSquareIcon.defaultProps = {
  xmlns: "http://www.w3.org/2000/svg",
  width: "16",
  height: "16",
  fill: "none",
  viewBox: "0 0 16 16"
};

export var IconWithoutSize = function IconWithoutSize() {
  return /*#__PURE__*/_jsx(Icon, {
    data: ArrowUpRightFromSquareIcon
  });
};
// #sourceMappingURL=CrmSelectDefaultOption.js.map

@arteria32 arteria32 force-pushed the fix/icons-svgr-data-props branch from d056ff9 to 55006e2 Compare July 29, 2024 06:01
@amje amje changed the title fix(Icon): use React.createElement on creating SVG component fix(Icon): correctly parse function with default props Jul 30, 2024
@amje amje merged commit b7eef14 into gravity-ui:main Jul 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants