-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Plugin: Try API for making experiments limited to WordPress codebase #41278
Conversation
Size Change: +563 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
@gziolo I like that it enables the usage of the I wonder whether a webpack plugin could add these transforms in a way that keeps the existing code working? |
Yes, there are Webpack plugins to do that, like this or this. Not sure if that approach will work in all instances, though. Maybe you can try using a regexp like Or just change the |
This is exactly the idea that Grzegorz was entertaining in conversation with me earlier today. :) Or, more realistically, something like |
|
@@ -1,2 +1,13 @@ | |||
export * from './components'; | |||
export { store } from './store'; | |||
|
|||
export const secretKey = process.env.WP_SECRET_KEY; |
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 possible, probable even, I am totally missing something but wouldn't this lead to the problem of plugins being able to import the secret key?
I ask this:
- knowing this is a really early POC so maybe I should wait
- acknowledging Adam's earlier comment about the overhead.
It's not that I don't trust plugins not to import the secret key if it's available, but it kind of is.
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 the way I understand this is that it solves the problem of experiments not being exposed in wp.*
JavaScript globals. Plus the key even if you get it, it changes with every release, therefore you'll never be able to rely on it for more than a few months. This is a great leap compared to how __experimental*
is within a simple wp.*
call AND sometimes it remains for years unchanged, therefore "reliable".
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 looks like @draganescu answered the question. This is the related part I included in the description:
What matters is that the
secretKey
is not available to everyone throughwp
global. The value assigned to thesecretKey
gets generated on every build. In practice, it means it will change with every new public release for WordPress so developers won't be able to hardcode this version. We can think about an even more elaborate approach if a stable key for a single WP release version is still a concern.
The trick is that secretKey
is bundled with webpack into every entry point that consumes the experimental/internal/private APIs. So in practice, secretKey
would never be available from WordPress core (and Gutenberg) in the browser.
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.
Thanks both. I was confused between internal exports for the package to use, vs external exports to the wp
global.
I think I have it now, thanks for your patience explaining.
This or |
I've turned this over in my head while on holidays and I don't think it's suitable after all. Changing the secret key each time the bundles are build will result in two things:
I kind of wish this hadn't occurred to me but it's probably a good thing in the long run. |
@peterwilsoncc It could be a version hash or something else equally stable across all builds of a given release.
Stable for just the major versions, then? |
I wonder if this would lead to the same problem: plugin authors would hard code a range of keys for various releases. I do appreciate you rubber ducking this, though. Thanks.
I've followed up my comment from above, the exact version was WP 5.7.1. |
Perhaps! At the same time, even with a system where a new key is generated per build, plugin authors could create an automation that keeps track of all the newly published keys. Ultimately, it boils down to the question: where do we set the line? To me, having a per-release key doesn't sound too bad as long as everything around it screams "do not use this key, this is intentionally hard," I'm thinking about:
...and so on |
Thank you for all the comments. I just wanted to emphasize that I intended to gather feedback on whether we should further explore the idea of creating a programmatic way to prevent accidental usage of internal Gutenberg APIs in WordPress core. Once we are all on the same page, we can start thinking about more creative ways to make that happen that are fully compatible with WordPress core requirements. Currently, it doesn't seem like the idea has enough support in the ongoing discussion in #40316. |
Absolutely understood. This is a |
This PR was opened only to show the concept. I don't plan to continue working on it in the close future. |
Here's an alternative idea – each package would register itself as follows: const ACCESS_TOKEN = {}; // An object with unique identity
registerPackage( '@wordpress/data', module.exports, ACCESS_TOKEN );
Different packages would use their own getPackage( '@wordpress/data', ACCESS_TOKEN ).__experimentalAPI
Here's a proof of concept: const CORE_PACKAGES_NAMES = [ '@wordpress/data', '@wordpress/core-data', '@wordpress/element', '@wordpress/edit-post', '@wordpress/edit-widgets' ];
const accessTokens = [];
const privatePackages = {};
export function registerPackage( packageName, package, accessToken ) {
if ( ! CORE_PACKAGES_NAMES.includes( packageName ) ) {
throw new Error(
`Cannot register a non-WordPress package ${ packageName }.`
);
}
if ( packageName in privatePackages ) {
throw new Error( `Package ${ packageName } is already registered.` );
}
accessTokens.push( accessToken );
privatePackages[ packageName ] = package;
}
export function getPackage( packageName, accessToken ) {
if ( ! ( packageName in privatePackages ) ) {
throw new Error( `Package ${ packageName } is not registered yet.` );
}
if ( ! accessTokens.includes( accessToken ) ) {
return null;
}
return privatePackages[ packageName ];
}
const ACCESS_TOKEN = {
i_understand_this_gives_me_access_to_internal_code_that_may_be_removed_at_any_time: true,
i_want_my_plugin_to_break_on_the_next_wordpress_release: true
} To me, that's enough to claim these private functions are internal and not a subject to BC guarantees. Thoughts @gziolo @peterwilsoncc @mcsf @talldan @noisysocks @azaozz ? |
Actually, perhaps even the following deterrent would suffice? // in @wordpress/data
export const getInternalFunctions( consent ) {
// ...
}
// In another package
import { __dangerousGetInternalAPIs } from '@wordpress/data';
const { __experimentalSomething } = __dangerousGetInternalAPIs({
i_want_my_plugin_to_break_on_the_next_wordpress_release: true,
current_wordpress_release: '6.0' // is this too much?
}); |
@adamziel, this is a very promising idea. Let me summarize how I understand your proposal:
Very clever. Instead of having a single global access token that needs to be obscured, we would use truly internal access tokens generated on the fly for every core package. |
I think this is really promising: the more difficult it is for an extender to access the API, the clearer the warning of upcoming change. |
I turned that idea into a PR – let's discuss there: #43386 |
What?
Based on the discussion from #40316.
Builds on the idea shared by @noisysocks in #40316 (comment):
I went for a simpler solution, for now, to explore how we can build a formal API that makes it possible to use all experimental APIs inside the Gutenberg plugin and WordPress core without exposing them to plugins and themes.
Why?
To make experimental APIs limited to usage only in WordPress codebase.
How?
I use one of the existing packages that are used internally -
@wordpress/interface
. It's a less important part at the moment where to put the code. What matters is that thesecretKey
is not available to everyone throughwp
global. The value assigned to thesecretKey
gets generated on every build. In practice, it means it will change with every new public release for WordPress so developers won't be able to hardcode this version. We can think about an even more elaborate approach if a stable key for a single WP release version is still a concern.For the Gutenberg plugin we can use a predictable
secrectKey
, for example:GUTENBERG
to allow plugins to use experimental APIs only with the Gutenberg plugin installed.Defining experiments in the package
With that, we have a formal API to define an experiment (example for
@wordpress/block-editor
package):which currently would be code as:
Consuming experiments
To use the experimental method we need to access the exposed
__experiments
helper function and combine it with a secret key that is available only internally in the bundled code:which is equivalent to the usage today: