Skip to content
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

Components: Remove "Experimental" designation #59418

Open
5 of 13 tasks
mirka opened this issue Feb 27, 2024 · 7 comments
Open
5 of 13 tasks

Components: Remove "Experimental" designation #59418

mirka opened this issue Feb 27, 2024 · 7 comments
Assignees
Labels
[Package] Components /packages/components [Priority] High Used to indicate top priority items that need quick attention [Type] Code Quality Issues or PRs that relate to code quality [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@mirka
Copy link
Member

mirka commented Feb 27, 2024

What problem does this address?

We used to designate a component as "Experimental" as a kind of backdoor to the back-compat policy, but since the policy was made more explicit and the Private API system was introduced, the "Experimental" has become meaningless. We will need to keep supporting them as regular, shipped components.

What is your proposed solution?

  1. Add non-prefixed exports for all the __experimental prefixed exports. (We will still need to keep the prefixed versions for back-compat.)
  2. Update all documentation so they don't include __experimental in the import statements.
  3. In Storybook, move all the "Components (Experimental)" items into the main "Components" group. (Use the Storybook redirect mechanism in Storybook: Add mechanism to redirect moved stories #59181 to maintain permalinks.)

Note

This particular issue is only for the components themselves, not the __experimental props, which should be addressed separately.

PRs:

@youknowriad
Copy link
Contributor

I think we should be careful here. Experimental APIs were introduced indeed in a time where they were not considered stable and any new experimental APIs will be considered stable but there's still an exception for the old experimental APIs. We have more room there to think about each component in isolation, how much it's used, whether it has an alternative, whether we should keep it or deprecate it.

In other words, I don't think we should be blindly renaming these components, we should only rename and stabilize the ones we're certain that we want to keep.

@mirka
Copy link
Member Author

mirka commented Apr 22, 2024

Yes, we will be using some discretion on a per-component basis, taking into account any alternatives and current usage levels 👍

@fullofcaffeine
Copy link
Member

fullofcaffeine commented Apr 23, 2024

Thanks for sharing your thoughts, @youknowriad! I'll keep what you said in mind as I work through these, but could you clarify the following points?

how much it's used

What criterion do you have in mind here, and what would be the actions for each case? E.g for components that are not being used, should we just remove them or perhaps we should remove the __experimental prefix without keeping the backwards-compatibility deprecated export?

How do we confidently gauge the usage frequency of components? We can check references in the GB app/repo, but there might be consumers in third-party packages we are not aware of.

whether it has an alternative, whether we should keep it or deprecate it.

Alternatives within our own library of components? How do we decide between one of another? I assume API design/simplicity, features, and code quality/maintainability (i.e., is it a newer version?).

Thanks!

@youknowriad
Copy link
Contributor

How do we confidently gauge the usage frequency of components?

In general I'd recommend using wpdirectory.net and check whether any plugins use the components (you have to dig through the results as some are false positives, plugins bundling Gutenberg).

I would recommend a deprecation for all the components regardless of the results:

  • If there's a component that we want to remove and its usage is very low/inexistant, we should write a Dev note about it and add a "version" (like 4 WP versions from now or so)
  • If a component is used, we'll probably have to keep it in the code base forever as deprecated (or at least for a very long time) but a Dev note can also help reduce its usage.

Before adding deprecations, we should first remove all existing usages from our own code base though.

@fullofcaffeine
Copy link
Member

If there's a component that we want to remove and its usage is very low/inexistant, we should write a Dev note about it and add a "version" (like 4 WP versions from now or so)

Can you expand on the "add a "version" (like 4 WP versions from now or so)" part? Not sure I follow.

If a component is used, we'll probably have to keep it in the code base forever as deprecated (or at least for a very long time) but a Dev note can also help reduce its usage.

Makes sense. I think this is essentially what we're doing here: #60913 (for example). The only difference (apart from the lack of a dev note, see below) is that we're also deprecating using @deprecate tsdoc annotation in the same changeset, and you're suggesting only adding that after we change the imports within the GB codebase, right?

write a Dev note about it

How do I add a dev note? It's different from a changelog, right?

@youknowriad
Copy link
Contributor

Can you expand on the "add a "version" (like 4 WP versions from now or so)" part? Not sure I follow.

There's a "version" argument you can add the "deprecated" function call to mark the version where the API is supposed to be removed. For instance 4 versions from now would be "6.9"

Makes sense. I think this is essentially what we're doing here: #60913 (for example). The only difference (apart from the lack of a dev note, see below) is that we're also deprecating using @deprecate tsdoc annotation in the same changeset, and you're suggesting only adding that after we change the imports within the GB codebase, right?

Yes, and we should be calling "deprecated" function as well to log the deprecation message in the console for plugin authors still using the API.

How do I add a dev note? It's different from a changelog, right?

Changelog is targeted towards package users, Dev notes are targeted towards WP plugins developers using the APIs through the scripts in WP. They're published before every WP major release. For PRs that require dev notes, you don't have to write the dev notes today, but you should be adding a "Needs Dev Note" label and you'll get pinged later when it's time to write the dev note.

@mirka
Copy link
Member Author

mirka commented May 23, 2024

@fullofcaffeine and I agreed on having some kind of prioritization here since there is a lot of work to be done. I attempted to group the experimental components into categories to help with priority and efficiency.

Controls

  • AlignmentMatrixControl
  • BorderBoxControl
  • BorderControl
  • BoxControl
  • DimensionControl
  • InputControl
  • NumberControl
  • ToggleGroupControl
  • UnitControl

Layout

  • Divider
  • Elevation
  • Grid
  • HStack
  • Scrollable
  • Spacer
  • Surface
  • VStack
  • View
  • ZStack

Typography

  • Heading
  • Text
  • Truncate

Misc

  • ConfirmDialog
  • DropdownMenuV2
  • ItemGroup
  • Navigator
  • ProgressBar
  • Tabs
  • ToolsPanel
  • TreeGrid

I think a good order would be to start with the Controls, since those are pretty straightforward and high impact. There are some prop tweaks that I want to discuss, and I'll note them in the individual PRs.

Next group would be Misc.

Layout and Typography, we should to first discuss and align on a long-term vision (with @DaniGuardiola too) before we decide what to do with individual components. At the moment, there is not enough clarity to say whether we want these types of components at all, and even if we do, what the APIs should look like ideally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Priority] High Used to indicate top priority items that need quick attention [Type] Code Quality Issues or PRs that relate to code quality [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Development

No branches or pull requests

3 participants