Skip to content

Conversation

lencioni
Copy link
Contributor

@lencioni lencioni commented Mar 20, 2017

Currently, when transforming an SVG file that has attributes on the
top-level <svg> element, the transformed component ends up looking
something like this:

_extends({
  xmlns: 'http://www.w3.org/2000/svg',
  viewBox: '0 0 1000 1000'
}, props),

this adds the overhead of _extends. I think it might be a better to
instead move these values into defaultProps on the generated
component.

Here's the diff of the result:

diff --git a/master.js b/default-props.js
index cb187ed..e62cca0 100644
--- a/master.js
+++ b/default-props.js
@@ -9,8 +9,6 @@ exports.MyClassIcon = undefined;
 
 var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }();
 
-var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };
-
 exports.MyFunctionIcon = MyFunctionIcon;
 
 var _react = require('react');
@@ -29,12 +27,7 @@ var MySvg = function () {
   function MySvg(props) {
     return _react2['default'].createElement(
       'svg',
-      _extends({
-        dataName: 'Livello 1',
-        id: 'Livello_1',
-        viewBox: '0 0 151.57 151.57',
-        xmlns: 'http://www.w3.org/2000/svg'
-      }, props),
+      props,
       _react2['default'].createElement('title', null),
       _react2['default'].createElement('circle', {
         cx: '1038.5',
@@ -78,6 +71,12 @@ var MySvg = function () {
   return MySvg;
 }();
 
+MySvg.defaultProps = {
+  dataName: 'Livello 1',
+  id: 'Livello_1',
+  viewBox: '0 0 151.57 151.57',
+  xmlns: 'http://www.w3.org/2000/svg'
+};
 function MyFunctionIcon() {
   return _react2['default'].createElement(MySvg, null);
 }

Closes #7

Currently, when transforming an SVG file that has attributes on the
top-level `<svg>` element, the transformed component ends up looking
something like this:

```js
_extends({
  xmlns: 'http://www.w3.org/2000/svg',
  viewBox: '0 0 1000 1000'
}, props),
```

this adds the overhead of _extends. I think it might be a better to
instead move these values into `defaultProps` on the generated
component.

Closes airbnb#7
@kesne kesne merged commit c03fd77 into airbnb:master Sep 26, 2017
atkinchris added a commit to atkinchris/babel-plugin-inline-react-svg that referenced this pull request Aug 8, 2020
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`: airbnb#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`).
vieira added a commit to vieira/babel-plugin-inline-react-svg that referenced this pull request Aug 30, 2024
Drop defaultProps from the generated component as they are now
deprecated and will be removed in React 19.

They were added in airbnb#8 due to concerns about the overhead of `_extends`
although no benchmarks were provided and it is likely that difference
would be marginal where it matters.

Fixes: airbnb#25
vieira added a commit to vieira/babel-plugin-inline-react-svg that referenced this pull request Aug 30, 2024
Drop defaultProps from the generated component as they are now
deprecated and will be removed in React 19.

They were added in airbnb#8 due to concerns about the overhead of `_extends`
although no benchmarks were provided and it is likely that difference
would be marginal where it matters.

Fixes: airbnb#126
atkinchris added a commit to atkinchris/babel-plugin-inline-react-svg that referenced this pull request Sep 10, 2024
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`: airbnb#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`).
atkinchris added a commit to atkinchris/babel-plugin-inline-react-svg that referenced this pull request Sep 10, 2024
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`: airbnb#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`).
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.

2 participants