-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Deprecations: Update versions supposed to be removed in WordPress 6.2 #60061
base: trunk
Are you sure you want to change the base?
Conversation
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@@ -29,7 +29,6 @@ function UnforwardedIconButton( | |||
deprecated( 'wp.components.IconButton', { | |||
since: '5.4', | |||
alternative: 'wp.components.Button', | |||
version: '6.2', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm removing the version for the IconButton. It seems like a very small wrapper and it's a bit hard to search in wpdirectory about potential usage.
@@ -66,7 +66,7 @@ function deprecateComponent( name, Wrapped, staticsToHoist = [] ) { | |||
deprecated( 'wp.editor.' + name, { | |||
since: '5.3', | |||
alternative: 'wp.blockEditor.' + name, | |||
version: '6.2', | |||
version: '6.8', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated all the "refactor editor to block-editor" APIs that happened on WP 5.3 to 6.8. I kind of randomly picked that number because there's still some usage for these APIs but removing them at some point will have a somewhat good impact on potentially bundle size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's almost 3kb for these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the refactor from the make post The Block Editor JavaScript module in 5.2?
It would be good to cut them loose (because it's 3kb plus the block-editor package when it may not be needed). I've been wondering if the editor package could be lazy loaded if these functions are used but not sure how to implement it. Adam Z is usually pretty good at this sort of magic though :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The editor package is the main package that powers the post and site editors, so it's not something that can be lazy loaded.
The deprecated functions may be, but it would a breaking change anyway since folks expect most of these to be synchronous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the refactor from the make post The Block Editor JavaScript module in 5.2?
yes
@@ -9,5 +9,5 @@ export { default as DotTip } from './components/dot-tip'; | |||
deprecated( 'wp.nux', { | |||
since: '5.4', | |||
hint: 'wp.components.Guide can be used to show a user guide.', | |||
version: '6.2', | |||
version: '6.8', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterwilsoncc When do you think is the right time to remove this one? It is fairly low impact because it's not loaded by default but I also don't really expect much usage at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think nux will need to be no-opped in this repo in order to safely deprecate it. Last I checked usage was pretty low (the wpdirectory search is down as I type this so I can't recheck).
I needed to restore it last time as it was causing packages to be downgraded in wordpress-develop when running the sync-gutenberg-packages
script & could foresee a mistake happening during a security release.
If it can be no-opped without any dependencies then I think we'll be able to safely deprecate the package as it won't trigger version constraints for older versions of other packages.
If it needs dependencies even while no-opped then I think it will need to sit around until the syncing script is refactored to account for it in wordpress-develop, and the publishing script is refactored in this repo.
Hopefully that all makes sense.
I'm still not sure there is any advantage in deprecating the package though. For WP it's not used, and sits there without being enqueued. For a plugin that is using it then it serves a purpose. Core still registeres jquery-form
for this purpose, even though it's not used by WP Core, only by plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not the most important one to remove for sure. I was just thinking that the usage is probably inexistant at this point. I don't care much though.
I think nux will need to be no-opped
What does "no-opped" mean? Functions and components that do nothing? Something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does "no-opped" mean? Functions and components that do nothing?
Yes, that's correct. This would need to include accessing the data store too.
I'm guessing this PR might trigger the discussion again about a "defined policy to deprecate and remove APIs". I'll preempt that I think:
|
Size Change: -6 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
@youknowriad Acknowledging I've seen this but I'm ignoring anything that isn't going to hit WordPress 6.5 until after the release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some replies to your original threads below.
I'm kind of inclined just to remove the hard deprecation numbers for stable APIs as we've never hit them as targets. Even for experimental APIs it's been a very rare occurrence.
@@ -66,7 +66,7 @@ function deprecateComponent( name, Wrapped, staticsToHoist = [] ) { | |||
deprecated( 'wp.editor.' + name, { | |||
since: '5.3', | |||
alternative: 'wp.blockEditor.' + name, | |||
version: '6.2', | |||
version: '6.8', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the refactor from the make post The Block Editor JavaScript module in 5.2?
It would be good to cut them loose (because it's 3kb plus the block-editor package when it may not be needed). I've been wondering if the editor package could be lazy loaded if these functions are used but not sure how to implement it. Adam Z is usually pretty good at this sort of magic though :)
@@ -9,5 +9,5 @@ export { default as DotTip } from './components/dot-tip'; | |||
deprecated( 'wp.nux', { | |||
since: '5.4', | |||
hint: 'wp.components.Guide can be used to show a user guide.', | |||
version: '6.2', | |||
version: '6.8', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think nux will need to be no-opped in this repo in order to safely deprecate it. Last I checked usage was pretty low (the wpdirectory search is down as I type this so I can't recheck).
I needed to restore it last time as it was causing packages to be downgraded in wordpress-develop when running the sync-gutenberg-packages
script & could foresee a mistake happening during a security release.
If it can be no-opped without any dependencies then I think we'll be able to safely deprecate the package as it won't trigger version constraints for older versions of other packages.
If it needs dependencies even while no-opped then I think it will need to sit around until the syncing script is refactored to account for it in wordpress-develop, and the publishing script is refactored in this repo.
Hopefully that all makes sense.
I'm still not sure there is any advantage in deprecating the package though. For WP it's not used, and sits there without being enqueued. For a plugin that is using it then it serves a purpose. Core still registeres jquery-form
for this purpose, even though it's not used by WP Core, only by plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good and soon we are going to release WP 6.6 I guess it is time to merge this PR 👍
@jorgefilipecosta I don't see any value in merging this until we have a unified back-compat discussion on make/core (see my comments on #61099) as it's unlikely these changes will be made in the versions identified. The private APIs feature has made the issue with |
What?
WordPress 6.5 is going to be released next week and we still have some deprecations in our code base saying that the APIs will be removed in 6.2. That's obviously a bad signal to send.
I'm going to try to do a review of these APIs (version per version) to see which ones should be delayed, remove the version entirely or remove the API.
No real impact here but I expect that we'd need to discuss some of these.