From a6de2dacb63e36572a2167b928418bdc39f3a9c2 Mon Sep 17 00:00:00 2001 From: Katja Lutz Date: Mon, 27 Apr 2020 21:01:54 +0200 Subject: [PATCH] fix: wrap svg component directly with memo/forwardRef (#440) (#441) --- .../src/__snapshots__/index.test.js.snap | 81 +++++++++------- .../src/index.test.js | 11 +++ .../src/util.js | 94 +++++++++---------- .../cli/src/__snapshots__/index.test.js.snap | 42 +++------ .../src/__snapshots__/convert.test.js.snap | 12 +-- 5 files changed, 117 insertions(+), 123 deletions(-) diff --git a/packages/babel-plugin-transform-svg-component/src/__snapshots__/index.test.js.snap b/packages/babel-plugin-transform-svg-component/src/__snapshots__/index.test.js.snap index 39c0ee90..b852111b 100644 --- a/packages/babel-plugin-transform-svg-component/src/__snapshots__/index.test.js.snap +++ b/packages/babel-plugin-transform-svg-component/src/__snapshots__/index.test.js.snap @@ -90,27 +90,22 @@ export default SvgComponent;" exports[`plugin javascript with "ref" and "expandProps" option expands props 1`] = ` "import * as React from \\"react\\"; -function SvgComponent({ - svgRef, - ...props -}) { +function SvgComponent(props, svgRef) { return ; } -const ForwardRef = React.forwardRef((props, ref) => ); +const ForwardRef = React.forwardRef(SvgComponent); export default ForwardRef;" `; exports[`plugin javascript with "ref" option adds ForwardRef component 1`] = ` "import * as React from \\"react\\"; -function SvgComponent({ - svgRef -}) { +function SvgComponent(props, svgRef) { return ; } -const ForwardRef = React.forwardRef((props, ref) => ); +const ForwardRef = React.forwardRef(SvgComponent); export default ForwardRef;" `; @@ -127,18 +122,30 @@ function SvgComponent({ export default SvgComponent;" `; -exports[`plugin javascript with both "memo" and "ref" option wrap component in "React.memo" and "React.forwardRef" 1`] = ` +exports[`plugin javascript with "titleProp" and "expandProps" adds "titleProp", "titleId" props and expands props 1`] = ` "import * as React from \\"react\\"; function SvgComponent({ - svgRef + title, + titleId, + ...props }) { return ; } -const MemoSvgComponent = React.memo(SvgComponent); -const ForwardRef = React.forwardRef((props, ref) => ); -export default ForwardRef;" +export default SvgComponent;" +`; + +exports[`plugin javascript with both "memo" and "ref" option wrap component in "React.memo" and "React.forwardRef" 1`] = ` +"import * as React from \\"react\\"; + +function SvgComponent(props, svgRef) { + return ; +} + +const ForwardRef = React.forwardRef(SvgComponent); +const MemoForwardRef = React.memo(ForwardRef); +export default MemoForwardRef;" `; exports[`plugin typescript custom templates support basic template 1`] = ` @@ -214,34 +221,23 @@ export default SvgComponent;" exports[`plugin typescript with "ref" and "expandProps" option expands props 1`] = ` "import * as React from \\"react\\"; -interface SVGRProps { - svgRef?: React.Ref -} -function SvgComponent({ - svgRef, - ...props -}: React.SVGProps & SVGRProps) { +function SvgComponent(props: React.SVGProps, svgRef?: React.Ref) { return ; } -const ForwardRef = React.forwardRef((props, ref: React.Ref) => ); +const ForwardRef = React.forwardRef(SvgComponent); export default ForwardRef;" `; exports[`plugin typescript with "ref" option adds ForwardRef component 1`] = ` "import * as React from \\"react\\"; -interface SVGRProps { - svgRef?: React.Ref -} -function SvgComponent({ - svgRef -}: SVGRProps) { +function SvgComponent(props: {}, svgRef?: React.Ref) { return ; } -const ForwardRef = React.forwardRef((props, ref: React.Ref) => ); +const ForwardRef = React.forwardRef(SvgComponent); export default ForwardRef;" `; @@ -262,19 +258,32 @@ function SvgComponent({ export default SvgComponent;" `; -exports[`plugin typescript with both "memo" and "ref" option wrap component in "React.memo" and "React.forwardRef" 1`] = ` +exports[`plugin typescript with "titleProp" and "expandProps" adds "titleProp", "titleId" props and expands props 1`] = ` "import * as React from \\"react\\"; interface SVGRProps { - svgRef?: React.Ref + title?: string, + titleId?: string, } function SvgComponent({ - svgRef -}: SVGRProps) { + title, + titleId, + ...props +}: React.SVGProps & SVGRProps) { return ; } -const MemoSvgComponent = React.memo(SvgComponent); -const ForwardRef = React.forwardRef((props, ref: React.Ref) => ); -export default ForwardRef;" +export default SvgComponent;" +`; + +exports[`plugin typescript with both "memo" and "ref" option wrap component in "React.memo" and "React.forwardRef" 1`] = ` +"import * as React from \\"react\\"; + +function SvgComponent(props: {}, svgRef?: React.Ref) { + return ; +} + +const ForwardRef = React.forwardRef(SvgComponent); +const MemoForwardRef = React.memo(ForwardRef); +export default MemoForwardRef;" `; diff --git a/packages/babel-plugin-transform-svg-component/src/index.test.js b/packages/babel-plugin-transform-svg-component/src/index.test.js index 311007eb..d7c6ffeb 100644 --- a/packages/babel-plugin-transform-svg-component/src/index.test.js +++ b/packages/babel-plugin-transform-svg-component/src/index.test.js @@ -62,6 +62,17 @@ describe('plugin', () => { }) }) + describe('with "titleProp" and "expandProps"', () => { + it('adds "titleProp", "titleId" props and expands props', () => { + const { code } = testPlugin(language)('', { + state: { componentName: 'SvgComponent' }, + expandProps: true, + titleProp: true, + }) + expect(code).toMatchSnapshot() + }) + }) + describe('with "expandProps"', () => { it('add props', () => { const { code } = testPlugin(language)('', { diff --git a/packages/babel-plugin-transform-svg-component/src/util.js b/packages/babel-plugin-transform-svg-component/src/util.js index 0369e44c..b956238c 100644 --- a/packages/babel-plugin-transform-svg-component/src/util.js +++ b/packages/babel-plugin-transform-svg-component/src/util.js @@ -72,17 +72,6 @@ function getSvgPropsTypeAnnotation(t) { export const getProps = ({ types: t }, opts) => { const props = [] - if (opts.ref) { - props.push( - t.objectProperty( - t.identifier('svgRef'), - t.identifier('svgRef'), - false, - true, - ), - ) - } - if (opts.titleProp) { props.push( t.objectProperty( @@ -103,53 +92,62 @@ export const getProps = ({ types: t }, opts) => { ) } - if (opts.expandProps) { + if (opts.expandProps && props.length > 0) { props.push(t.restElement(t.identifier('props'))) } - if (props.length === 0) { - return null - } + const propsArgument = + props.length > 0 ? t.objectPattern(props) : t.identifier('props') + let propsTypeAnnotation + if (props.length > 0) { + propsTypeAnnotation = genericTypeAnnotation(t.identifier('SVGRProps')) - if (props.length === 1 && opts.expandProps) { - return addTypeAnotation( - t.identifier('props'), - typeAnnotation(getSvgPropsTypeAnnotation(t)), - opts, - ) + if (opts.expandProps) { + propsTypeAnnotation = intersectionTypeAnnotation([ + getSvgPropsTypeAnnotation(t), + propsTypeAnnotation, + ]) + } + } else { + propsTypeAnnotation = opts.expandProps + ? getSvgPropsTypeAnnotation(t) + : t.objectPattern([]) } - return addTypeAnotation( - t.objectPattern(props), - typeAnnotation( - opts.expandProps - ? intersectionTypeAnnotation([ - getSvgPropsTypeAnnotation(t), - genericTypeAnnotation(t.identifier('SVGRProps')), - ]) - : genericTypeAnnotation(t.identifier('SVGRProps')), - ), + const typedPropsArgument = addTypeAnotation( + propsArgument, + typeAnnotation(propsTypeAnnotation), opts, ) -} -export const getInterface = ({ types: t }, opts) => { - if (!opts.typescript) return null - const properties = [] + const args = [] + if (opts.expandProps || props.length > 0 || opts.ref) + args.push(typedPropsArgument) + if (opts.ref) { - properties.push( - objectTypeProperty( - t.identifier('svgRef'), + const refArgument = t.identifier(opts.typescript ? 'svgRef?' : 'svgRef') + const typedRefArgument = addTypeAnotation( + refArgument, + typeAnnotation( genericTypeAnnotation( qualifiedTypeIdentifier(t.identifier('React'), t.identifier('Ref')), typeParameters([ genericTypeAnnotation(t.identifier('SVGSVGElement')), ]), ), - true, ), + opts, ) + + args.push(typedRefArgument) } + + return args +} + +export const getInterface = ({ types: t }, opts) => { + if (!opts.typescript) return null + const properties = [] if (opts.titleProp) { properties.push( objectTypeProperty(t.identifier('title'), t.identifier('string'), true), @@ -198,21 +196,15 @@ export const getExport = ({ template }, opts) => { plugins.push('typescript') } - if (opts.memo) { - const nextExportName = `Memo${exportName}` - result += `const ${nextExportName} = React.memo(${exportName})\n\n` - exportName = nextExportName - } - if (opts.ref) { const nextExportName = `ForwardRef` - let refType = '' - - if (opts.typescript) { - refType = ': React.Ref' - } + result += `const ${nextExportName} = React.forwardRef(${exportName})\n\n` + exportName = nextExportName + } - result += `const ${nextExportName} = React.forwardRef((props, ref${refType}) => <${exportName} svgRef={ref} {...props} />)\n\n` + if (opts.memo) { + const nextExportName = `Memo${exportName}` + result += `const ${nextExportName} = React.memo(${exportName})\n\n` exportName = nextExportName } diff --git a/packages/cli/src/__snapshots__/index.test.js.snap b/packages/cli/src/__snapshots__/index.test.js.snap index 4d5e74e8..bfb70700 100644 --- a/packages/cli/src/__snapshots__/index.test.js.snap +++ b/packages/cli/src/__snapshots__/index.test.js.snap @@ -235,7 +235,7 @@ exports[`cli should support various args: --native --ref 1`] = ` "import * as React from 'react' import Svg, { Path } from 'react-native-svg' -function SvgFile({ svgRef, ...props }) { +function SvgFile(props, svgRef) { return ( @@ -243,9 +243,7 @@ function SvgFile({ svgRef, ...props }) { ) } -const ForwardRef = React.forwardRef((props, ref) => ( - -)) +const ForwardRef = React.forwardRef(SvgFile) export default ForwardRef " @@ -339,7 +337,7 @@ export default SvgFile exports[`cli should support various args: --ref 1`] = ` "import * as React from 'react' -function SvgFile({ svgRef, ...props }) { +function SvgFile(props, svgRef) { return ( @@ -347,9 +345,7 @@ function SvgFile({ svgRef, ...props }) { ) } -const ForwardRef = React.forwardRef((props, ref) => ( - -)) +const ForwardRef = React.forwardRef(SvgFile) export default ForwardRef " @@ -407,17 +403,14 @@ export default SvgFile exports[`cli should support various args: --typescript --ref --title-prop 1`] = ` "import * as React from 'react' interface SVGRProps { - svgRef?: React.Ref; title?: string; titleId?: string; } -function SvgFile({ - svgRef, - title, - titleId, - ...props -}: React.SVGProps & SVGRProps) { +function SvgFile( + { title, titleId, ...props }: React.SVGProps & SVGRProps, + svgRef?: React.Ref, +) { return ( ) => ( - -)) +const ForwardRef = React.forwardRef(SvgFile) export default ForwardRef " @@ -442,14 +433,11 @@ export default ForwardRef exports[`cli should support various args: --typescript --ref 1`] = ` "import * as React from 'react' -interface SVGRProps { - svgRef?: React.Ref; -} -function SvgFile({ - svgRef, - ...props -}: React.SVGProps & SVGRProps) { +function SvgFile( + props: React.SVGProps, + svgRef?: React.Ref, +) { return ( @@ -457,9 +445,7 @@ function SvgFile({ ) } -const ForwardRef = React.forwardRef((props, ref: React.Ref) => ( - -)) +const ForwardRef = React.forwardRef(SvgFile) export default ForwardRef " diff --git a/packages/core/src/__snapshots__/convert.test.js.snap b/packages/core/src/__snapshots__/convert.test.js.snap index e1d7f883..86ce2877 100644 --- a/packages/core/src/__snapshots__/convert.test.js.snap +++ b/packages/core/src/__snapshots__/convert.test.js.snap @@ -192,7 +192,7 @@ exports[`convert config should support options 8 1`] = ` "import * as React from 'react' import Svg, { G, Path } from 'react-native-svg' -function SvgComponent({ svgRef, ...props }) { +function SvgComponent(props, svgRef) { return ( ( - -)) +const ForwardRef = React.forwardRef(SvgComponent) export default ForwardRef " `; @@ -218,7 +216,7 @@ export default ForwardRef exports[`convert config should support options 9 1`] = ` "import * as React from 'react' -function SvgComponent({ svgRef, ...props }) { +function SvgComponent(props, svgRef) { return ( ( - -)) +const ForwardRef = React.forwardRef(SvgComponent) export default ForwardRef " `;