Fix popover deprecations

[talldan]

What?

Some popover deprecations were incorrect. This PR should fix them. I've added the backport label as the deprecation messages may be confusing for developers.

The changes are:

• range - this prop seems to have no effect, and has been that way for quite some time. It looks like it was removed in b9871c6, before Gutenberg landed in core, it seems like no functionality would ever have existed in WP core, so I've decided to remove it. It's amazing it has persisted in the codebase!
• __unstableShift - this prop has never existed in WordPress core. It has been deprecated in the plugin for several releases (albeit with a confusing version number), but I think it's fine to now remove this.
• anchorRef, anchorRect, getAnchorRect. These are stable props in WP core, so technically they cannot be removed. The deprecation message was stating they'd be removed in 6.3, so I've changed the deprecation message.

Testing Instructions

• Popovers should continue to work.

[ciampo]

Until today I wasn't aware (and I believe @mirka wasn't either?) that props could not be removed from React components even after marking them as deprecated and giving them a minimum notice period before removal.

Specifically for Popover, not being able to delete some of those props will leave the component very bloated and harder to maintain (see the logic regarding all of the anchor-related values).

We even published a Dev Note where these props have been marked for removal — does it mean that the dev note is also written incorrectly?

Also, related to the whole __experimental debate that happened recently — with these anti-deletion rules in place, we are basically forced to use add __unstable and __experimental props and components, so that we can have a way out.

(to make this clear — none of what I wrote above is a complaint aimed at you, Dan! You actually happened to notice those policies, which we had missed/misinterpreted).

[talldan]

Specifically for Popover, not being able to delete some of those props will leave the component very bloated and harder to maintain

The policy definitely presents some real challenges, and as you say why there's so much unstable and experimental code.

We even published a Dev Note where these props have been marked for removal — does it mean that the dev note is also written incorrectly?

We can always update that and say that some aspects of it have been revised, I think that would be ok.

[noisysocks]

These changes and @talldan's reasoning look right to me!

Until today I wasn't aware (and I believe @mirka wasn't either?) that props could not be removed from React components even after marking them as deprecated and giving them a minimum notice period before removal.

To be clear this only applies to APIs that have made it into Core.

My understanding is that the Gutenberg deprecation policy is:

• Stable APIs can be marked deprecated and removed after two plugin releases.
• Experimental/unstable APIs can be removed without notice.

And that the current Core depreciation policy is:

• Stable APIs can be marked deprecated but never removed.
• Being debated: Experimental/unstable APIs can be removed without notice.

That last point is what was being debated in Make/Core. I don't think there's been an "official" decision about it but I suspect we'll soon (already have?) start treating experimental APIs in Core as if they were stable. There's a new @wordpress/experiments package intended for APIs that should be hidden from Core, though I'm not sure if it works with props.

[cbravobernal]

May be too late to land this for 6.1?
Breaking changes a week before the main release sounds a little risky 😅

[ciampo]

Code change LGTM, thank you @talldan .

May be too late to land this for 6.1?
Breaking changes a week before the main release sounds a little risky 😅

These should be "safe" breaking changes, as Dan highlighted above:

• the range prop didn't have any effect on the component already
• the __unstableShift prop was introduced after the WordPress 6.0 release in this PR, and therefore never made its way to Core

[ciampo]

To be clear this only applies to APIs that have made it into Core.

My understanding is that the Gutenberg deprecation policy is:

• Stable APIs can be marked deprecated and removed after two plugin releases.
• Experimental/unstable APIs can be removed without notice.

And that the current Core depreciation policy is:

• Stable APIs can be marked deprecated but never removed.
• Being debated: Experimental/unstable APIs can be removed without notice.

I understand these policies and the reasoning behind them, but at the same time I do think that the forced accumulation of legacy code can really end up encumbering us developers working on Gutenberg daily, and potentially cause more harm to the developers ecosystem than a controlled deprecation / deletion (as said before, see for example the amount of experimental/unstable APIs introduced to circumvent the problem).

I wish there was a protocol to safely make this sort of breaking changes when they are needed.

There's a new @wordpress/experiments package intended for APIs that should be hidden from Core, though I'm not sure if it works with props.

It would be quite problematic if that didn't work with props — show is the best person to talk to, in order make sure they are supported?

[talldan]

Breaking changes a week before the main release sounds a little risky 😅

@c4rl0sbr4v0 They're not breaking changes.

[ciampo]

Actually @talldan , do we need to apply the same treatment to the useAnchorRef hook that was also marked as deprecated and for deletion ?

I imagine that it is ok to still mark the __unstableForcePosition prop on Popover for deletion, given its experimental nature?

We even published a Dev Note where these props have been marked for removal — does it mean that the dev note is also written incorrectly?

We can always update that and say that some aspects of it have been revised, I think that would be ok.

Hey @bph , I made some updates to the Dev note post — could you please take a look? I wasn't sure if/how/where to add a note about the fact that the post has been updated since its initial publication.

[bph]

@ciampo To update the Dev Note, you add "Updated: date" and describe briefly the change and point to the section where the update happened.

Here is an example:

Would that work?

[ciampo]

Would that work?

Yep! I went ahead and updated the post — thank you!

[talldan]

This this may have missed the boat for 6.1. I think it had to be merged yesterday before RC3 was cut. The unfortunate side effect is that __unstableShift will make its way into WP Core, but at least it'll be deprecated from day 1 (and potentially we will want to keep it in the codebase now until 6.3).

I've labeled this PR now for backport to 6.1.1.

@ciampo It may be worth adding to the dev note that the deprecation messages will be updated in 6.1.1.

[talldan]

Actually @talldan , do we need to apply the same treatment to the useAnchorRef hook that was also marked as deprecated and for deletion ?

Yes, I think the version will have to be removed from that deprecation message.

[ciampo]

@ciampo It may be worth adding to the dev note that the deprecation messages will be updated in 6.1.1.

Updated the dev note, thank you.

Yes, I think the version will have to be removed from that deprecation message.

Sure, I will spin up a quick PR to change the deprecation message around useAnchorRef (edit: #45302)