-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
(experiment)(wip) Components: Abstract/namespace all related Navigator
components in the root component
#60926
base: trunk
Are you sure you want to change the base?
Conversation
Navigator
components in the root compomentNavigator
components in the root component
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
f760382
to
9c493f8
Compare
970d466
to
ee903b9
Compare
ee903b9
to
a4c4fe0
Compare
Flaky tests detected in 8a625f8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9615299743
|
…nt for simpler export/import
a4c4fe0
to
8a625f8
Compare
Taking over this PR. Looking at the pending items left in the PR:
That is currently being discussed in #60927
I'm not sure if bundlers would be able to tree-shake different components sub-items (ie — if a consumer imports I don't think it matters much in this scenario, because almost every subcomponent is necessary when using
I decided to rename the At the same time, the considerations about tree shaking above could also represent an important factor to consider. |
If you do this: import { Navigator } from '@wordpress/components';
return <Navigator.Button />; then the entire In theory, the |
I experimented a few months ago and confirmed that the major bundlers won't tree-shake dot notation subcomponents 🥲 Though, it's not technically impossible (e.g. rollup/rollup#2201). In the end though, I don't think this will be a blocker for us adopting dot notation subcomponenting. We just need to be mindful of this when designing APIs for components that have a large, optional dependency in one of it subcomponents. An example that comes to mind would be a tooltip. As long as we design the API so the large dependency can be kept separate, it should be fine. // ❌ Bundles an optional dep into an untree-shakeable subcomponent
<Foo.Bar tooltipText="foo" />
// 🟢 Keeps the optional dep "externalized"
<Tooltip text="foo">
<Foo.Bar />
</Tooltip>
// 🟢 Or alternatively
<Foo.Bar tooltip={ <Tooltip text="foo" /> } /> |
Navigator
#60927What / How
Namespace all children sub-components in the root
NavigatorProvider
component. Also, export aNavigator
alias forNavigatorProvider
to make it more semantically sound. Keep exporting all individual components as-is for backwards compatibility.Why
Closely related (children) components always used together should ideally be namespaced into the root parent component. This provides a cleaner API that's easier to use and understand.
TODO:
Navigator
(abstract away the provider part from the name, to simplify) or asNavigatorProvider
or both.