Repository navigation
Private APIs: Deprecate the '@wordpress/dataviews' opt-in, scheduling removal for WordPress 7.4 - #82231
Private APIs: Deprecate the '@wordpress/dataviews' opt-in, scheduling removal for WordPress 7.4#82231oandregal wants to merge 3 commits into
Conversation
… removal for WordPress 7.4 Move '@wordpress/dataviews' from the allowed core modules to a new DEPRECATED_CORE_MODULES list. Opting in as a deprecated module still works, so previously published npm copies keep loading, but it logs a deprecation warning when SCRIPT_DEBUG is enabled. A unit test starts failing when the WordPress 7.4 release cycle begins, prompting the actual removal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LjZ4WCUJRKsVT8Uds1gKgp
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LjZ4WCUJRKsVT8Uds1gKgp
oandregal
added
the
[Type] Code Quality
|
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. |
|
Size Change: +98 B (0%) Total Size: 7.92 MB 📦 View Changed
|
18.1 is the DataViews version that no longer uses private APIs Co-authored-by: André <583546+oandregal@users.noreply.github.com>
| it( 'Should be removed when the WordPress version scheduled for their removal is reached', () => { | ||
| // The newest lib/compat/wordpress-X.Y directory marks the WordPress | ||
| // release Gutenberg is currently targeting. | ||
| const compatDir = path.join( __dirname, '../../../../lib/compat' ); | ||
| const targetedVersion = fs | ||
| .readdirSync( compatDir ) | ||
| .map( ( name ) => name.match( /^wordpress-(\d+)\.(\d+)$/ ) ) | ||
| .filter( Boolean ) | ||
| .map( ( match ) => [ Number( match[ 1 ] ), Number( match[ 2 ] ) ] ) | ||
| .sort( ( a, b ) => a[ 0 ] - b[ 0 ] || a[ 1 ] - b[ 1 ] ) | ||
| .pop(); | ||
|
|
||
| const dueForRemoval = []; | ||
| for ( const [ moduleName, { removal } ] of Object.entries( | ||
| DEPRECATED_CORE_MODULES | ||
| ) ) { | ||
| const [ major, minor ] = removal.split( '.' ).map( Number ); | ||
| const isReached = | ||
| targetedVersion[ 0 ] > major || | ||
| ( targetedVersion[ 0 ] === major && | ||
| targetedVersion[ 1 ] >= minor ); | ||
| if ( isReached ) { | ||
| dueForRemoval.push( | ||
| `The "${ moduleName }" entry in DEPRECATED_CORE_MODULES was scheduled ` + | ||
| `for removal in WordPress ${ removal }, and the ` + | ||
| `${ targetedVersion[ 0 ] }.${ targetedVersion[ 1 ] } release cycle has started. ` + | ||
| `Remove the entry and its deprecation warning, unless old copies of the ` + | ||
| `module are still in significant use by plugins.` | ||
| ); | ||
| } | ||
| } | ||
| expect( dueForRemoval ).toEqual( [] ); | ||
| } ); |
There was a problem hiding this comment.
Given the long timelines to make this change, I though having something that acts as an alarm would be of help. Having a failing test is a better signal than relying on people's memory.
However, I don't like that this test starts failing when we create the lib/compat/wordpress-X.Y/ folder, as that requires the PR that creates the folder to also remove the deprecated entry.
Happy to hear alternatives.
There was a problem hiding this comment.
Happy to hear alternatives.
I implemented an alternative way back in #5491 for exactly this reason but I see that we gave up on this in #68590 (in part due to changes in how we track WordPress version and not Gutenberg). For what it's worth, I felt and feel quite strongly that we should be consistent about this and enforce it project-wide.
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| `The private APIs opt-in of the module "${ moduleName }" is deprecated since WordPress ${ since } ` + | ||
| `and will stop working in WordPress ${ removal }. ${ note }` | ||
| ); |
There was a problem hiding this comment.
Can we use @wordpress/deprecated for this?
And if we do, can we update the typings of DEPRECATED_CORE_MODULES value to reuse the existing type signature from @wordpress/deprecated accordingly?
| // eslint-disable-next-line @eslint-community/eslint-comments/disable-enable-pair | ||
| /* eslint-disable @wordpress/wp-global-usage */ |
There was a problem hiding this comment.
I'd suggest just adding the pairing at the bottom of the file than to disable the disable-enable rule, which exists largely for this pattern. Ideally we could scope the disabling rule more precisely to where we expect it to exist.
Given that we also disable this in the implementation, I'd wonder if it's something where the package or file pattern should be exempt from the rule. Though to be honest, I don't really understand the purpose of the @wordpress/wp-global-usage rule to begin with.
| warnedDeprecatedModules.clear(); | ||
| } | ||
|
|
||
| export { DEPRECATED_CORE_MODULES }; |
There was a problem hiding this comment.
Could we export this at the variable assignment? Not sure if we have this as a normal convention elsewhere.
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Meant to approve with those comments, nothing blocking here 👍
What?
Follow up to #82221 (see this discussion). Moves
@wordpress/dataviewsfrom the list of core modules allowed to use private APIs to a new list of deprecated modules. Opting in as a deprecated module still works, but logs a deprecation warning, and its removal is scheduled for WordPress 7.4.Why?
@wordpress/dataviewscopies published to npm before v18 call the private APIs opt-in when they load, so a plugin bundling one of them would break the moment we remove the entry. We still want to remove it eventually, and developers need a heads-up on how to prepare (update the npm dependency to v18 or newer).How?
Adds a
DEPRECATED_CORE_MODULESlist to@wordpress/private-apis. Modules on it still getlock/unlock, but a warning is logged once per module whenSCRIPT_DEBUGis enabled, naming the removal version and the fix. A new unit test starts failing as soon as alib/compat/wordpress-7.4directory appears, so the removal cannot be forgotten; actual usage of old copies in the plugin directory should still be checked before removing.Testing Instructions
Run
npm run test:unit -- packages/private-apis. To see the warning in a browser, open the editor in a dev environment withSCRIPT_DEBUGenabled and runwp.privateApis.__dangerousOptInToUnstableAPIsOnlyForCoreModules( 'I acknowledge private features are not for use in themes or plugins and doing so will break in the next version of WordPress.', '@wordpress/dataviews' )in the console: it returns the lock/unlock utilities and logs the deprecation warning once.Use of AI Tools
This PR was written with the help of Claude Code, based on the discussion linked above. The changes were reviewed, tested, and are owned by the author.