Connectors: Replace plugin.slug with plugin.file#11400
Connectors: Replace plugin.slug with plugin.file#11400jorgefilipecosta wants to merge 5 commits intoWordPress:trunkfrom
Conversation
…ation Use the plugin's main file path (e.g. 'akismet/akismet.php') instead of just the slug. This lets the script module data function use file_exists() and is_plugin_active() directly, removing the get_plugins() slug-to-file map. The frontend derives the slug from the file path when needed.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
mukeshpanchal27
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Have you consider single file plugin (hello.php)?
This should work, too. Worth including in the PHPDoc for completeness. |
|
@jorgefilipecosta, to test it in the browser we need to land Gutenberg changes and update the hash to have JS changes compatible? |
Exactly or we can test this together with Gutenberg plugin activated. |
src/wp-includes/connectors.php
Outdated
| 'pluginFile' => $is_installed | ||
| ? ( str_ends_with( $plugin_file, '.php' ) ? substr( $plugin_file, 0, -4 ) : $plugin_file ) | ||
| : null, | ||
| 'file' => str_ends_with( $file, '.php' ) ? substr( $file, 0, -4 ) : $file, |
There was a problem hiding this comment.
The file key here has .php stripped (e.g. ai-provider-for-anthropic/plugin), but the registration file key includes it (e.g. ai-provider-for-anthropic/plugin.php). Using the same key name for two different values in different contexts could be confusing.
Consider either:
- Keeping the
.phpextension in the JS output so it matches the registration value exactly, or - Renaming the output key to something like
slugorpluginBasenameto signal it's a derived value
There was a problem hiding this comment.
The existing test test_register_includes_plugin_data in tests/phpunit/tests/connectors/wpConnectorRegistry.php:297 still uses 'slug' => 'my-plugin' and passes because the registry stores whatever keys are provided in the plugin array without validation. This should be updated to use 'file' to match the new contract:
$args['plugin'] = array( 'file' => 'my-plugin/my-plugin.php' );$this->assertSame( array( 'file' => 'my-plugin/my-plugin.php' ), $result['plugin'] );This also flags that the registry might benefit from cherry-picking known properties from the plugin array (i.e., only keeping file) rather than storing whatever is passed in. That way invalid keys would be silently dropped and the stored data would always match the documented shape. It still works properly because we do that sanitization when passing data to the client.
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Summary
plugin.slugwithplugin.file(the full plugin file path, e.g.akismet/akismet.php) in connector registrationsget_plugins()slug-to-file map, usefile_exists()andis_plugin_active()directlyisInstalledto the frontend plugin dataTicket: https://core.trac.wordpress.org/ticket/65002
Test plan