Provide option for spreading props rather than static assignment #86
Conversation
|
(In fact, I'm getting the warning using Next.js today: |
|
That's the plan, but since it's a terrible idea, I think it's worth waiting until it ships, and the React team responds to the inevitable fallout, before treating that as gospel. |
|
hey @atkinchris was this issue resolved or did you find a workaround? i am currently experiencing the same error all over my next app due to deprecated defaultProps being used when transforming svgs |
|
If it's just next.js issuing that warning, I'd suggest filing an issue with them to remove it, since it's clearly premature. |
|
With the release of React 18.3, the @ljharb, would you be open to the Out of curiosity, what kind of fallout are you anticipating? What negative side effects should developers be concerned about? The only two discussions on |
|
There’s a lot of benefits from propTypes and defaultProps, both in runtime effect and tooling introspection/composition, but since react has continued with this plan anyways, we might as well add the option. |
|
Any plans on releasing this? |
|
Just wanted to mention that we upgraded to React 18 from 17, and I'm seeing 50+ warnings for our icon imports. An option makes a lot of sense, and we are not using next.js , just ejected CRA |
|
Hello, should we expect an update to fix this issue? |
|
It'd be really great to have a solution or workaround for this issue, because it makes it hard to recognize more relevant errors and warnings. |
|
Just to add on the pile. It would be really nice if this can be handled. :) Please? Pretty please? |
This comment was marked as spam.
This comment was marked as spam.
|
This PR needs a rebase. |
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`).
This provides an option (
spreadDefaultProps) to spread the extra props from the top levelsvgelement onto the props object, rather than statically assigning them asdefaultProps. This gives users an optional trade off for_spreadperformance (as raised in the PR that originally setupdefaultProps) against being able to tree shake the generated components (which is prevented if they have static assignments).The
READMEhas been updated, and a sanity test scenario added.This also corrects some bugs which arise when
opts.SVG_DEFAULT_PROPS_CODEis 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 whenopts.SVG_DEFAULT_PROPS_CODEis not assigned, and therefore theSVG_NAME.defaultProps = SVG_DEFAULT_PROPS_CODEtemplate string is never included - however, theSVG_DEFAULT_PROPS_CODEis still passed.The second is using
replaceWithversusreplaceWithMultiplewhen there are multiple nodes to be replaced. This is currently predicated onopts.SVG_DEFAULT_PROPS_CODE- which is not accurate. It should be predicated on if there are multiple nodes (svgReplacement.length > 1).