From 4676590f06a79be2094beb73e4ec3e59e30d57bc Mon Sep 17 00:00:00 2001 From: Chris Atkin Date: Sat, 8 Aug 2020 11:56:04 +0100 Subject: [PATCH] Provide option for spreading props rather than static assignment This provides an option (`spreadDefaultProps`) to spread the extra props from the top level `svg` element onto the props object, rather than statically assigning them as `defaultProps`. This gives users an optional trade off for `_spread` performance (as raised in the PR that originally setup `defaultProps`: https://github.com/airbnb/babel-plugin-inline-react-svg/pull/8) against being able to tree shake the generated components (which is prevented if they have static assignments). The `README` has been updated, and a sanity test scenario added. This also corrects some bugs which arise when `opts.SVG_DEFAULT_PROPS_CODE` is not assigned. The first is that Babel will throw an error when a substitution key is provided but not used, even if it's value is `undefined`. This happens when `opts.SVG_DEFAULT_PROPS_CODE` is not assigned, and therefore the `SVG_NAME.defaultProps = SVG_DEFAULT_PROPS_CODE` template string is never included - however, the `SVG_DEFAULT_PROPS_CODE` is still passed. The second is using `replaceWith` versus `replaceWithMultiple` when there are multiple nodes to be replaced. This is currently predicated on `opts.SVG_DEFAULT_PROPS_CODE` - which is not accurate. It should be predicated on if there are multiple nodes (`svgReplacement.length > 1`). --- README.md | 1 + src/index.js | 31 ++++++++++++++++++++++++------- test/sanity.js | 26 +++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 71dd723..72adeb0 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,7 @@ npm install --save-dev babel-plugin-inline-react-svg - *`ignorePattern`* - A pattern that imports will be tested against to selectively ignore imports. - *`caseSensitive`* - A boolean value that if true will require file paths to match with case-sensitivity. Useful to ensure consistent behavior if working on both a case-sensitive operating system like Linux and a case-insensitive one like OS X or Windows. +- *`spreadDefaultProps`* - A boolean value that if `true` will spread additional props, rather than setting them as `defaultProps` static assignment. This is important for tree shaking, as static assignments prevent efficient dead code removal. - *`svgo`* - svgo options (`false` to disable). Example: ```json { diff --git a/src/index.js b/src/index.js index ed9a63f..1dddd91 100644 --- a/src/index.js +++ b/src/index.js @@ -39,9 +39,26 @@ export default declare(({ `; if (SVG_NAME !== 'default') { - return template(namedTemplate)({ SVG_NAME, SVG_CODE, SVG_DEFAULT_PROPS_CODE }); + const substitutions = { SVG_CODE, SVG_NAME }; + + // If a key is present in the substitutions object, but is unused in the template, even if + // even if it's value is undefined, Babel will throw an error. + if (SVG_DEFAULT_PROPS_CODE) { + substitutions.SVG_DEFAULT_PROPS_CODE = SVG_DEFAULT_PROPS_CODE; + } + + return template(namedTemplate)(substitutions); } - return template(anonymousTemplate)({ SVG_CODE, SVG_DEFAULT_PROPS_CODE, EXPORT_FILENAME }); + + const substitutions = { SVG_CODE, EXPORT_FILENAME }; + + // If a key is present in the substitutions object, but is unused in the template, even if + // even if it's value is undefined, Babel will throw an error. + if (SVG_DEFAULT_PROPS_CODE) { + substitutions.SVG_DEFAULT_PROPS_CODE = SVG_DEFAULT_PROPS_CODE; + } + + return template(anonymousTemplate)(substitutions); }; function applyPlugin(importIdentifier, importPath, path, state, isExport, exportFilename) { @@ -91,8 +108,9 @@ export default declare(({ EXPORT_FILENAME: exportFilename, }; - // Move props off of element and into defaultProps - if (svgCode.openingElement.attributes.length > 1) { + // Move props off of element and into defaultProps, but only if + // the "spreadDefaultProps" argument is not true + if (state.opts.spreadDefaultProps !== true && svgCode.openingElement.attributes.length > 1) { const keepProps = []; const defaultProps = []; @@ -108,11 +126,10 @@ export default declare(({ opts.SVG_DEFAULT_PROPS_CODE = t.objectExpression(defaultProps); } - if (opts.SVG_DEFAULT_PROPS_CODE) { - const svgReplacement = buildSvg(opts); + const svgReplacement = buildSvg(opts); + if (svgReplacement.length > 1) { path.replaceWithMultiple(svgReplacement); } else { - const svgReplacement = buildSvg(opts); path.replaceWith(svgReplacement); } file.get('ensureReact')(); diff --git a/test/sanity.js b/test/sanity.js index b04db4c..4ae812e 100644 --- a/test/sanity.js +++ b/test/sanity.js @@ -13,8 +13,20 @@ function assertReactImport(result) { } } +function assertSpreadProps(result) { + const hasSpreadProps = result.code.match(/_extends\(\{.*"'data-name'": "Livello 1".*\}, props\)/gs); + if (!hasSpreadProps) { + throw new Error('Spread props were not found'); + } + + const hasDefaultProps = result.code.match(/\.defaultProps/g); + if (hasDefaultProps) { + throw new Error('Default props were found, when spread should have been used instead'); + } +} + function validateDefaultProps(result) { - if (!(/'data-name':/g).test(result.code)) { + if (!(/'data-name':?/g).test(result.code)) { throw new Error('data-* props need to be quoted'); } } @@ -177,3 +189,15 @@ transformFile('test/fixtures/test-export-default-as.jsx', { if (err) throw err; console.log('test/fixtures/test-export-default-as.jsx', result.code); }); + +transformFile('test/fixtures/test-export-default-as.jsx', { + presets: ['airbnb'], + plugins: [ + [inlineReactSvgPlugin, { spreadDefaultProps: true }], + ], +}, (err, result) => { + if (err) throw err; + console.log('test/fixtures/test-export-default-as.jsx', result.code); + validateDefaultProps(result); + assertSpreadProps(result); +});