-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use defaultProps instead of adding props to the svg #7
Comments
I'm not sure how that'd work out... we don't know the props ahead of time, so we can't statically set defaultProps. Here's an example of how this all shakes out: import MySvg from './file.svg';
const Test = () => <MySvg prop="red" />; This turns into: const MySvg = (props) => <svg {...props} />;
const Test = () => <MySvg prop="red" />; Feel free to re-open if I'm mistaken. |
The case I'm talking about here is if the <svg foo="bar"></svg> import MySvg from './file.svg'; into: const MySvg = (props) => <svg foo="bar" {...props} />; What I am suggesting is to instead convert these static props that are in the SVG file to defaultProps instead, like this: const MySvg = (props) => <svg {...props} />;
MySvg.defaultProps = {
foo: "bar",
}; |
Ah okay, that makes more sense. I can look into it, and I'd accept a PR that does that. Although I'm not super convinced that |
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
Currently, the transformed component ends up looking something like this:
this adds the overhead of
_extends
. I think it might be a better to instead move these values into defaultProps on the generated component.The text was updated successfully, but these errors were encountered: