paragon
paragon copied to clipboard
Migrate off of compound components to standalone components
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 include From.Group
component into the resulting bundle even if From.Group
is not explicitly used in the code, same goes for all Form.<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 the Form
object so you have to bring them all into the bundle if you import Form
), 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
- [ ] Ensure that Paragon no longer uses compound components in the code and instead exports only standalone components (e.g. `Form` should no longer have `Form.Group` component, instead this component should be exported under `FormGroup` name)
- [ ] Create a CLI command that replaces all compound components' usages with their standalone counterpart (e.g. `Form.Group` is replaced with `FormGroup` across all code)
- [ ] Verify all react-bootstrap imports pull from standalone components rather than root import
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 possible, release a new feat/minor version of Paragon that includes deprecation warnings when consumers use a compound component to suggest using a standalone component instead.
- When the stated DEPR date comes, we can then delete all the compound components, leaving only the standalone components and release a new breaking change.
If we do provide an automated CLI tool to do this:
- Implement CLI tool to migrate from compound components to associated standalone components when run in a consumer's repo. See https://github.com/openedx/paragon/pull/2460 for a foundational start at a singular
paragon
CLI that supports multiple features. - Release breaking change with the removal of all compound components, with the release notes mentioning the CLI tool.
(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 Card
and CardStatus
it would still bring all Card.<name>
components into the bundle, and CardStatus
would be included twice which is even worse than what we have now). So I think full deprecation is the only way here.
Perhaps we go through the official DEPR process?
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:
-
Alert
, bringsAlert.Heading
andAlert.Link
from bootstrap which are very lightweight and usually used together withAlert
, so no big deal; -
Card
, includes a lot of stuff... We do override most of its subcomponents though, e.g. we have our ownCard.Header
that replaces rect-bootstrap's one, BUT when we remove our ownCard.Header
and expose standaloneCardHeader
, react-bootstrap'sCard.Header
will still be included in the bundle. Which is bad, we will have 2 versions of header included. Considering that we don't use anything from react-bootstrap related toCard
except for the parent component which just acts as a wrapper, we really should replace it with our own implementation. I'll file a separate issue for that; -
Carousel
, includesCarousel.Item
andCarousel.Caption
which we currently override, they are also lightweight and used together; -
Dropdown
, has 6 compound components, they are also mostly lightweight and usually used together with the parent component. Might still be worth to implement our own version. -
Form
, has 8 compound components in it and we override only 5 of them I think. Seems to be worth to implement our own version, considering that we've built a ton of custom into this component already. (and all thatForm
component does is just wraps its children with<form />
tag, so should be fairly easy to remove react-bootstrap's dependency here); -
Figure
, has 2 compound components, look very lightweight; -
Nav
, has 2 compound components, look very lightweight, usually used together with the parent; -
Navbar
, has 4 compound components, not sure whether they usually used, also don't look too lightweight... might be worth refactoring this one; -
Popover
, has 2 compound components, usually used together, look lightweight; -
Toast
, has 2 compound components, our docs don't give any example on their usage, so they're probably never used. I suggest to update the docs so at least people use it. Implementing our ownToast
to remove compound components might not be easy / worth it.
All things considered, I think it's not too bad 🙂. We should definitely remove react-bootstrap's dependency from Card
and Form
components though, otherwise their compound components will start hurting us once we remove ours. (also, like 90% percent of these component are overridden by Paragon, it just doesn't make much sense to inherit from react-bootstrap anymore).
react-bootstrap reference: https://github.com/react-bootstrap/react-bootstrap/tree/v1.6.5/src (this is the current version we have in our master
branch)