Promote fetchLinkSuggestions to non-experimental API #40052
Conversation
getdave
added
[Type] Code Quality
|
Size Change: +3.04 kB (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
getdave
requested review from
draganescu,
talldan,
tellthemachines,
adamziel,
kevin940726,
noisysocks,
ellatrix and
nerrad
as code owners
| @@ -1,2 +1,2 @@ | |||
| export { default as __experimentalFetchLinkSuggestions } from './__experimental-fetch-link-suggestions'; | |||
| export { default as fetchLinkSuggestions } from './__experimental-fetch-link-suggestions'; | |||
There was a problem hiding this comment.
Let's rename the file name to fetch-link-suggestions.js too :)
There was a problem hiding this comment.
Along with the unit test file.
@kevin940726 Thanks good spot. I renamed things. Look better? |
| @@ -1,7 +1,7 @@ | |||
| /** | |||
There was a problem hiding this comment.
I think we forgot to delete this file 😅
There was a problem hiding this comment.
Noooooooo! How did this happen 🤦
Somehow missed first time around
|
Given how long this has been around we need to give some time for folks to adapt to the change. I'm going to hold on merging until after WP 6.0 and also going to raise in Core Editor chat. |
|
I reached out to a few of the Plugins in the directory that I could find who have used this API. I did this as a personal courtesy and not as part of any official process determined by Gutenberg Core. I'll give this a few days then merge. |
|
Given that this feature has been bundled with core, and is in use on ~800 000-900 000 active installs based off the WordPress.org repository, and an unknown amount of paid plugins or themes, is there any reason why an alias is not introduced to maintain backwards compatibility? |
This is a good point. You'll recognise my reticence to merge this is based primarily on concerns along the same lines you have mentioned. In general anything marked I believe there is a wider discussion happening about how we can avoid situations such as this occuring. However, in this case we are where we are. Let's see how other @WordPress/gutenberg-core team folks feel but it might be reasonable to introduce a deprecation notice and possibly retain an alias for |
|
Hi there! experimental APIs are not considered stable APIs and are documented as such. It's like So in that sense, there's no difference between how we approached things in Gutenberg and existing practices pre-gutenberg in Core. We also did similar renames a lot of times in the past (a quick search in the PR list can show these) which much impact. It is true that plugin and theme authors that want to use internal functions are using these and this won't change in the future regardless of what we do. and it's also not something new with Gutenberg as I said before. For me, I still think we should continue the current approach, it has proved to be good in reducing the impact, communicating things properly to the third-party developers and allowing us to only expose things as stable when we're ready to commit to them long term in terms of backward compatibility. (outside core internal usage). -- |
I understand you are saying that we should
That would seem to cause the least impact. Given how prevalent the usage of this API is I agree we should treat it as a special case.
Agreed. But as you say in certain cases it might be necessary. |
|
On a second look to the wp-directory results. I think most of the results are false positives:
Anyway, I don't really think this API is that impactful (I don't see it as an important one for third-parties either). It's probably better if we just contact these plugins to confirm that it's false positives. We've done this in the past. cc @annezazu |
|
FYI I think some of these instances could be based on using https://github.com/Automattic/isolated-block-editor/ as a base. I've contacted them and they've raised Automattic/isolated-block-editor#135. If we're not concerned then an alternative approach could be:
I wonder if @annezazu can reach out a bit and help identify the potential impact? The other side of the coin is, if there is a level of uncertainty about the impact, perhaps we should just add the deprecation anyway. If we're worried about bloat, we can just tidy up after ourselves once the deprecation period has ended as we did recently with Navigation Areas. |
|
I'm on it :) Contacted the following folks today and linked to this issue for good measure:
I'll loop back if I hear from them to confirm but, for now, consider proactive outreach done. |
There was a problem hiding this comment.
I'm approving since it works and is consistent with Gutenberg __experimental policy. That's separate from handling the communication. A quick WP directory search by __experimentalFetchLinkSuggestions revealed the following plugins:
- MailPoet – emails and newsletters in WordPress
- Gutenberg
- Pretty Links – Link Management, Branding, Tracking & Sharing Plugin
- Email Marketing Automation, Email Newsletter and CRM Plugin for WordPress by FluentCRM
- Kubio Page Builder
- Friends
- Gartentechnik.com
- Booking Weir
|
Quick note for transparency that MailPoet is part of Automattic where I work (along with you) and they too have been contacted by @getdave previously. I left them off my above list for that reason but it feels important to clarify that they have been reached out to. |
|
@youknowriad @adamziel I'm going to flag this once more in Core Editor chat this week and then look to merge. |
|
Here is where I flagged it again https://wordpress.slack.com/archives/C02QB2JS7/p1652884611338199 Unless anyone else objects or ✋ then I will merge this before the end of the week. |
|
I share @Clorith's concern. A sudden, hard deprecation of this in WordPress will break a lot of sites and the WordPress commitment to maintain backward compatibility. As the conversation about making the block-editor less experimental is ongoing, I think it would be provocative to hard deprecate prior to the resolution. Given the outreach to larger plugins in the plugin directory, of course the usage in the public code is going down but that's not indicative of custom themes, plugins, etc. |
There was a problem hiding this comment.
Marking this with a change request per my comment above.
I've created a PR with said changes: #41183.
|
Closing this one. |
youknowriad
deleted the
try/make-fetch-link-suggestions-non-experimental
branch
See https://make.wordpress.org/core/2022/04/25/editor-chat-agenda-27-april-2022/#comment-42966
What?
This PR updates
__experimentalFetchLinkSuggestionstofetchLinkSuggestionsthereby making it non-experimental.Why?
The API has been in Core Data for a few years now and it's had time to bed in.
Given that the API has been around for so long there is a good chance 3rd party Plugins will utilise the
__experimentalprefixed version. We should be clear to highlight this in release notes.How?
All instances of
__experimentalFetchLinkSuggestionsin Core renamed to remove prefix.Testing Instructions
Screenshots or screencast