Provide option for spreading props rather than static assignment

[atkinchris]

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) 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).

[jpveooys]

defaultProps is being deprecated on function components (https://github.com/facebook/react/pull/25699), therefore it would be good to land something like this.

(In fact, I'm getting the warning using Next.js today: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.)

[ljharb]

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.

[KodinManiac]

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

[ljharb]

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.

[connor-baer]

With the release of React 18.3, the defaultProps warning is now in a stable release of React and will affect many more developers.

@ljharb, would you be open to the spreadDefaultProps configuration option now, so developers can decide between the two approaches themselves? 🙏

Out of curiosity, what kind of fallout are you anticipating? What negative side effects should developers be concerned about? The only two discussions on defaultProps I could find on the original RFC seem to have been answered.

[ljharb]

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.

[einarq]

Any plans on releasing this?

[sedghi]

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

[olivierpascal]

Hello, should we expect an update to fix this issue?

[davilima6]

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.

[nemanjacosovic]

Just to add on the pile. It would be really nice if this can be handled. :) Please? Pretty please?

[GoudekettingRM]

Bump

[ljharb]

This PR needs a rebase.