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.

Sign up for GitHub

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

Jump to bottom

Components: Remove "experimental" designation #60837

Closed
wants to merge 6 commits into from
Closed

Components: Remove "experimental" designation #60837

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ce4f638
Remove "experimental" designation for `AlignmentMatrixControl`
fullofcaffeine Apr 17, 2024
51d389b
Remove "experimental" designation for `BorderBoxControl` and related …
fullofcaffeine Apr 17, 2024
ddaca25
Remove "experimental" designation for `BorderControl`
fullofcaffeine Apr 17, 2024
65049e0
Remove "experimental" designation for `BoxControl` and related function
fullofcaffeine Apr 17, 2024
ddf0218
Remove "experimental" designation for `ConfirmDialog`
fullofcaffeine Apr 18, 2024
752a07f
Remove "experimental" designation for `CustomSelectControl[V2]` and e…
fullofcaffeine Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-alignment-matrix-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { DOWN } from '@wordpress/keycodes';
import {
ToolbarButton,
Dropdown,
__experimentalAlignmentMatrixControl as AlignmentMatrixControl,
AlignmentMatrixControl,
} from '@wordpress/components';

const noop = () => {};
Expand Down
6 changes: 3 additions & 3 deletions packages/block-editor/src/components/global-styles/border-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
* WordPress dependencies
*/
import {
__experimentalBorderBoxControl as BorderBoxControl,
__experimentalHasSplitBorders as hasSplitBorders,
__experimentalIsDefinedBorder as isDefinedBorder,
BorderBoxControl,
hasSplitBorders,
isDefinedBorder,
__experimentalToolsPanel as ToolsPanel,
__experimentalToolsPanelItem as ToolsPanelItem,
__experimentalItemGroup as ItemGroup,
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/border.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { hasBlockSupport, getBlockSupport } from '@wordpress/blocks';
import { __experimentalHasSplitBorders as hasSplitBorders } from '@wordpress/components';
import { hasSplitBorders } from '@wordpress/components';
import { Platform, useCallback, useMemo } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks';
import { useSelect } from '@wordpress/data';
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/table/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
TextControl,
ToggleControl,
ToolbarDropdownMenu,
__experimentalHasSplitBorders as hasSplitBorders,
hasSplitBorders,
} from '@wordpress/components';
import {
alignLeft,
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/alignment-matrix-control/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ AlignmentMatrixControl components enable adjustments to horizontal and vertical

```jsx
import { useState } from 'react';
import { __experimentalAlignmentMatrixControl as AlignmentMatrixControl } from '@wordpress/components';
import { AlignmentMatrixControl } from '@wordpress/components';

const Example = () => {
const [ alignment, setAlignment ] = useState( 'center center' );
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/alignment-matrix-control/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import type { AlignmentMatrixControlProps } from './types';
* AlignmentMatrixControl components enable adjustments to horizontal and vertical alignments for UI.
*
* ```jsx
* import { __experimentalAlignmentMatrixControl as AlignmentMatrixControl } from '@wordpress/components';
* import { AlignmentMatrixControl } from '@wordpress/components';
* import { useState } from '@wordpress/element';
*
* const Example = () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/alignment-matrix-control/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { HStack } from '../../h-stack';
import type { AlignmentMatrixControlProps } from '../types';

const meta: Meta< typeof AlignmentMatrixControl > = {
title: 'Components (Experimental)/AlignmentMatrixControl',
title: 'Components/AlignmentMatrixControl',
component: AlignmentMatrixControl,
subcomponents: {
// @ts-expect-error - See https://github.com/storybookjs/storybook/issues/23170
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/border-box-control/border-box-control/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ show "Mixed" placeholder text.

```jsx
import { useState } from 'react';
import { __experimentalBorderBoxControl as BorderBoxControl } from '@wordpress/components';
import { BorderBoxControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';

const colors = [
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/border-box-control/border-box-control/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ const UnconnectedBorderBoxControl = (
* view's width input would show "Mixed" placeholder text.
*
* ```jsx
* import { __experimentalBorderBoxControl as BorderBoxControl } from '@wordpress/components';
* import { BorderBoxControl } from '@wordpress/components';
* import { __ } from '@wordpress/i18n';
*
* const colors = [
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/border-box-control/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import Button from '../../button';
import { BorderBoxControl } from '../';

const meta: Meta< typeof BorderBoxControl > = {
title: 'Components (Experimental)/BorderBoxControl',
title: 'Components/BorderBoxControl',
component: BorderBoxControl,
argTypes: {
onChange: { action: 'onChange' },
Expand Down
5 changes: 4 additions & 1 deletion packages/components/src/index.native.js
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

Let's not touch the *.native.js stuff for now, they are completely separate.

Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ export {

export { default as __experimentalStyleProvider } from './style-provider';
export { default as BaseControl } from './base-control';
export { hasSplitBorders as __experimentalHasSplitBorders } from './border-box-control/utils';
export {
hasSplitBorders as __experimentalHasSplitBorders,
hasSplitBorders,
} from './border-box-control/utils';
export { default as TextareaControl } from './textarea-control';
export { default as PanelBody } from './panel/body';
export { default as PanelActions } from './panel/actions';
Expand Down
9 changes: 8 additions & 1 deletion packages/components/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ export {
} from '@wordpress/primitives';

// Components.
export { default as __experimentalAlignmentMatrixControl } from './alignment-matrix-control';
export {
default as __experimentalAlignmentMatrixControl,
Copy link
Member

Choose a reason for hiding this comment

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

While I appreciate that we're keeping the __experimental exports for backward compatibility reasons, I wonder if we should deprecate __experimental imports now that the components are stable. That will nudge 3rd party developers to migrate to stable imports, otherwise they might be stuck with the __experimental ones forever

Copy link
Member

Choose a reason for hiding this comment

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

There are so many "shades" of deprecation — hard, soft, TS-only, TSDoc only... 😂 For these I think a TSDoc @deprecated Import AlignmentMatrixControl instead would be appropriate. Is that what you had in mind?

Copy link
Member

Choose a reason for hiding this comment

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

Yup, it's odd that deprecation can be so complex 😅

Anyway, imagine I'm a plugin dev that uses some experimental components in my plugin. How will a TSDoc @deprecated help me find out that I'm using a deprecated import and I have to replace it with the canonical stable import? Will I even be notified about it by my editor, or does that rely on me being very attentive to reading the type hints provided by my editor?

Copy link
Member Author

@fullofcaffeine fullofcaffeine Apr 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Will I even be notified about it by my editor, or does that rely on me being very attentive to reading the type hints provided by my editor?

I managed to make the @deprecate annotation work when it's added just before the actual export symbol, inside the export block:

export {
	/**
	 * @deprecated Since version X.X. Will be removed in version X.Y. Use `ConfirmDialog` instead.
	 */
	ConfirmDialog as __experimentalConfirmDialog,
	ConfirmDialog,
} from './confirm-dialog';

I can confirm it doesn't affect the second export there:

Screenshot 2024-04-18 at 12 51 30 p m

And of the @deprecated declaration, it will render like this in vscode (notice the strikethrough in the import and the warning in the IntelliSense popup):
Screenshot 2024-04-18 at 12 48 28 p m Screenshot 2024-04-18 at 12 49 04 p m

So I assume all editors/IDEs that support tsdoc will warn users about it.

If that's a good way to prevent developers from continuing to use it, that is another story. It might be good to do the deprecation gradually? First soft-deprecate using a tsdoc, announce in the changelog and perhaps a Make post, then in another (major?) version, completely remove the __experimental exports.

Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

And yeah, gradual deprecation sounds good to me. We should likely document the plan for that in the related issue, including some releases where we aim to further/hard deprecate.

Copy link
Member

Choose a reason for hiding this comment

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

Anyway, imagine I'm a plugin dev that uses some experimental components in my plugin. How will a TSDoc @deprecated help me find out that I'm using a deprecated import and I have to replace it with the canonical stable import? Will I even be notified about it by my editor, or does that rely on me being very attentive to reading the type hints provided by my editor?

My current stance on this is,

  1. Do we plan to hard deprecate it?
  2. Does switching to the new thing provide the consumer with actual benefits over the old thing?

If the answer is "no" to both questions, we shouldn't log a console warning and we shouldn't block the TS from compiling. We shouldn't demand a dev's attention for what is essentially a housekeeping detail. If they care about housekeeping details, they'll do it.

And to be clear, personally I don't think the __experimental exports are worth hard deprecating. It's virtually free to maintain this BC, compared to the cost on the ecosystem if we yank them.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, the complex deprecation discussion again 😅

I think we have far too many cases that are cheap/free to maintain, but because we have too many, it adds so much noise. I wish we had a clear deprecation policy that actually allows us to get rid of deprecated features at some point.

If I come back to planet Earth though and draw a line on the tradeoffs, you're right and it should be just fine to continue soft-deprecating as suggested.

AlignmentMatrixControl,
} from './alignment-matrix-control';
export {
default as Animate,
getAnimateClassName as __unstableGetAnimateClassName,
Expand All @@ -33,6 +36,10 @@ export {
hasSplitBorders as __experimentalHasSplitBorders,
isDefinedBorder as __experimentalIsDefinedBorder,
isEmptyBorder as __experimentalIsEmptyBorder,
BorderBoxControl,
hasSplitBorders,
isDefinedBorder,
isEmptyBorder,
Comment on lines +40 to +42
Copy link
Member

Choose a reason for hiding this comment

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

Ok, these three exports are sounding major alarm bells for me 😱 Not sure what's going on there. Let's keep these experimental for now so we can investigate and figure out a better way.

Copy link
Contributor

Choose a reason for hiding this comment

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

See: #61135

} from './border-box-control';
export { BorderControl as __experimentalBorderControl } from './border-control';
export {
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/global-styles/screen-block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { store as coreStore } from '@wordpress/core-data';
import {
PanelBody,
__experimentalVStack as VStack,
__experimentalHasSplitBorders as hasSplitBorders,
hasSplitBorders,
} from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';

Expand Down
6 changes: 5 additions & 1 deletion storybook/manager-head.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<script>
( function redirectIfStoryMoved() {
const PREVIOUSLY_EXPERIMENTAL_COMPONENTS = [ 'navigation' ];
const PREVIOUSLY_EXPERIMENTAL_COMPONENTS = [
'navigation',
'alignmentmatrixcontrol',
'borderboxcontrol'
];
const REDIRECTS = [
{
from: /\/components-deprecated-/,
Expand Down