-
Notifications
You must be signed in to change notification settings - Fork 66
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
Migrate off of compound components to standalone components #2464
Comments
Thanks for filing this issue, @viktorrusakov! I agree with the intent of this ticket. The alternative is to continue to export both compound components alongside their associated standalone tokens like we do today, and have the choice of which to use be up to the consumer. But, I feel this is less than ideal because it leaves a path for consumers to use known less performant/optimized code, so I agree we should plan on removing the compound components. Perhaps we go through the official DEPR process?: I'd be happy to take this part on. If we don't provide an automated CLI tool to do this:
If we do provide an automated CLI tool to do this:
(i.e., no deprecation warning period necessary if we have a CLI tool, imho) |
@adamstankiewicz I think exporting both compound and standalone versions of components is not an option also because parent component would always include all of its compound components even if standalone imports are used (e.g. if someone imports just
Yeah, I also was wondering if we should. Let's do it, I'll file the DEPR issue. I'd rather go with the CLI tool option. I would feel bad making consumers manually rename components 😅. Also this tool could be very useful in the future, seems worth creating it. |
@adamstankiewicz As for validating react-bootstrap imports - we almost always import standalone components, but there are some exceptions where we import a parent component that contains compound components 😔. These components are:
All things considered, I think it's not too bad 🙂. We should definitely remove react-bootstrap's dependency from react-bootstrap reference: https://github.com/react-bootstrap/react-bootstrap/tree/v1.6.5/src (this is the current version we have in our |
As was discovered by #2425, compound components that are extensively used by Paragon do not support tree-shaking (e.g., importing
Form
component would also includeFrom.Group
component into the resulting bundle even ifFrom.Group
is not explicitly used in the code, same goes for allForm.<subcomponentName>
components). This negatively impacts bundles sizes of consuming applications.It appears that there is no way to support tree-shaking for compound components (that's because all of the
Form.<subcomponentName>
components are part of theForm
object so you have to bring them all into the bundle if you importForm
), that's why popular component libraries (e.g. Material UI, Chakra UI) do not have compound components in their code, instead they utilize standalone components.We should remove compound components from Paragon to provide best performance possible to consumers. This will result in a breaking change, so we should also provide a CLI command to ease migration.
Tasks
The text was updated successfully, but these errors were encountered: