fix: wrap svg component directly with memo/forwardRef (#440)

[katywings]

Summary

Already described in: #440 - there is also a demo :D

Root cause for the typescript error

By wrapping the svg component in a new component at

svgr/packages/babel-plugin-transform-svg-component/src/util.js

Line 215 in 6e7e6b2

result += `const ${nextExportName} = React.forwardRef((props, ref${refType}) => <${exportName} svgRef={ref} {...props} />)\n\n`

the generic types of React.forwardRef loose the prop types from the svg component.

Solution

This fix directly wraps the component in React.forwardRef without something in between. Not wrapping the component in an empty proxy component also has a slight performance advantage. To accomplish the direct wrap with React.forwardRef I also had to change the order with React.memo: React.forwardRef has to be the first wrapper around the svg component because React.memo itself doesnt forward the ref further - the difference can be seen here: https://github.com/gregberge/svgr/pull/441/files#diff-fd642229e3068089af3cc1b24827673aR147

Output changes

To summarize what this changes in the svg component (taken from "plugin typescript with "ref" and "expandProps" option expands props 1" before/after snapshots):

OLD:

interface SVGRProps {
  svgRef?: React.Ref<SVGSVGElement>
}

function SvgComponent({
  svgRef,
  ...props
}: React.SVGProps<SVGSVGElement> & SVGRProps) {
  return <svg><g /></svg>;
}

const ForwardRef = React.forwardRef((props, ref: React.Ref<SVGSVGElement>) => <SvgComponent svgRef={ref} {...props} />);
export default ForwardRef;

NEW:

function SvgComponent(props: React.SVGProps<SVGSVGElement>, svgRef?: React.Ref<SVGSVGElement>) {
  return <svg><g /></svg>;
}

const ForwardRef = React.forwardRef(SvgComponent);
export default ForwardRef;

Of course this is only one of many option combinations that had slight changes as seen in the test commits, but it gives a good overview.

Test plan

I tested it with yarn test and also tried it out with several builds using feather-icons.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/gregberge/svgr/cb92iyfna
✅ Preview: https://svgr-git-fork-katywings-bugfix-forward-ref-expand-props.gregberge.now.sh

[katywings]

P.S: when I started with the fix I didn't think that I would have to make so many changes xD, but I just couldn't stop in the middle

[codecov]

Codecov Report

Merging #441 into master will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
+ Coverage   86.53%   86.60%   +0.07%     
==========================================
  Files          31       31              
  Lines         557      560       +3     
  Branches      153      155       +2     
==========================================
+ Hits          482      485       +3     
  Misses         60       60              
  Partials       15       15              

Impacted Files	Coverage Δ
...s/babel-plugin-transform-svg-component/src/util.js	95.58% <100.00%> (+0.20%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f30bb85...84370ea. Read the comment docs.

[gregberge]

Hello @katywings, it looks great! It simplifies code and fixes a bug, thank you!

[katywings]

@gregberge De nada! Thank you for merging it so soon :)