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

Removing Reakit Composite implementation #58620

Merged
merged 8 commits into from
Feb 2, 2024

Conversation

andrewhayward
Copy link
Contributor

What?

Removes the 'unstable' Reakit implementation of the Composite component group.

Why?

We need to remove Reakit from the codebase (#53278).

How?

  • Points default export to ./legacy implementation
  • Changes 'legacy' Storybook title to "Composite"
  • Removes deprecation messages from legacy (to be put back when current is available)
  • Removes unstable Reakit implementation

Testing Instructions

Open Storybook, and confirm that there are still two Composite entries ("Composite" and "Composite (V2)").

Storybook sidebar items, with "Composite (V2)" and "Composite" highlighted.

Note

There are no consumers of the Reakit implementation left in Gutenberg, so we have to trust that test coverage is sufficient that any switch is transparent for consumers.

@andrewhayward andrewhayward added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Feb 2, 2024
@andrewhayward andrewhayward requested a review from a team February 2, 2024 14:51
@andrewhayward andrewhayward self-assigned this Feb 2, 2024
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of comments, but overall the changes look good.

Should we also remove reakit in this PR, or do you prefer to do it in a follow-up?

(also cc @mirka for an extra pair of eyes)

packages/components/src/composite/index.ts Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Feb 2, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core SVN

If you're a Core Committer, use this list when committing to wordpress-develop in SVN:

Props: andrewhayward, mciampini, tyxla.

GitHub Merge commits

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: andrewhayward <andrewhayward@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ciampo ciampo requested a review from mirka February 2, 2024 15:17
@andrewhayward
Copy link
Contributor Author

Should we also remove reakit in this PR, or do you prefer to do it in a follow-up?

Personally I'd rather do it in a follow-up PR, if only to keep this one focused on Composite, but that's not a strongly-held opinion.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

packages/components/CHANGELOG.md Outdated Show resolved Hide resolved
@@ -28,6 +28,10 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also unrelated, but just spotted that we have 2 "Enhancements" sections.

@andrewhayward andrewhayward added [Type] Task Issues or PRs that have been broken down into an individual action to take and removed [Type] Code Quality Issues or PRs that relate to code quality labels Feb 2, 2024
@ciampo ciampo added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Feb 2, 2024
@ciampo
Copy link
Contributor

ciampo commented Feb 2, 2024

Adding the Needs dev note label.

Assuming that changes up to reakit removal will be part of the WordPress 6.5 release, and that the promotion of the new Composite component from private to public API will be part of the WordPress 6.6 release, we will need to separate dev notes:

  • one for the 6.5 release, highlighting that we removed reakit because of its incompatibility with newer versions of react and wrote a back-compat layer in ariakit which should cover standard use cases of the component (in theory no action required from consumers of the package). We could also anticipate that we're looking into exposing a new, stable version of the component written natively in ariakit.
  • one for the 6.6 release, where we'll announce the new, stable, ariakit-based Composite component, and the related deprecation of the __unstableComposite family of components.

cc @mirka and @bph

@ciampo ciampo merged commit 5f740d2 into trunk Feb 2, 2024
62 checks passed
@ciampo ciampo deleted the 56548/remove-reakit-composite-implementation branch February 2, 2024 17:17
@github-actions github-actions bot added this to the Gutenberg 17.7 milestone Feb 2, 2024
@andrewhayward
Copy link
Contributor Author

Dev note

Changes to the underlying Composite component implementation

WordPress 6.5 no longer includes the Reakit package, as it does not support React beyond v16. The __unstable* Composite component was built on this library, so has been internally refactored.

What does this mean for consumers?

The primary API has not changed, and can still be imported with no changes. For typical usage patterns, there should be no differences, and in most cases no action is required from consumers of the package.

However, composite state props must now be generated by the useCompositeState hook; they can no longer be provided by independent state logic. Composite state arguments have not changed though, and will continue to work as before.

import {
  __unstableComposite: Composite,
  __unstableCompositeGroup: CompositeGroup,
  __unstableCompositeItem: CompositeItem,
  __unstableUseCompositeState: useCompositeState
} from '@wordpress/components';

const state = useCompositeState({ ... });

// ✅ This will continue to work
...( <Composite { ...state } /> );

// ⛔️ This will no longer work
...( <Composite { ...state } currentId={ ... } /> );

Consumers can continue to either spread the state, or pass as a single state prop...

// ✅ Used by spreading the state
...(
  <Composite { ...state }>
    <CompositeGroup { ...state }>
      <CompositeItem { ...state }>
        { ... }
      </CompositeItem>
    </CompositeGroup>
  </Composite>
);

// ✅ Or with a single `state` prop
...(
  <Composite state={ state }>
    <CompositeGroup state={ state }>
      <CompositeItem state={ state }>
        { ... }
      </CompositeItem>
    </CompositeGroup>
  </Composite>
);

Because the shape of the returned composite state has changed, consumers can also now no longer destructure specific state props from useCompositeState.

// ✅ This will continue to work
const state = useCompositeState({ ... });

// ⛔️ This will no longer work
const { groups, items } = useCompositeState({ ... });

What's next?

We anticipate a new stable Composite component to be released in WordPress 6.6, along with the deprecation of this unstable version.

@ciampo ciampo mentioned this pull request Jul 19, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Note Requires a developer note for a major WordPress release cycle [Package] Components /packages/components [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants