Skip to content

Commit

Permalink
Provide option for spreading props rather than static assignment
Browse files Browse the repository at this point in the history
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`: #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`).
  • Loading branch information
atkinchris committed Aug 8, 2020
1 parent d94d268 commit 4676590
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
31 changes: 24 additions & 7 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 = [];

Expand All @@ -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')();
Expand Down
26 changes: 25 additions & 1 deletion test/sanity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Expand Down Expand Up @@ -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);
});

0 comments on commit 4676590

Please sign in to comment.