Conversation
…opt-out Gutenberg introduced a system of sharing private APIs in #46131. One of the safeguards is a check preventing the same module from opting-in twice so that contributors cannot easily gain access by pretending to be a core module. That safeguard is only meant for WordPress core and not for the released @WordPress packages. However, right now it is opt-out and must be explicitly disabled by developers wanting to install the @WordPress packages. Let's make it opt-out instead. This commit makes the check opt-in rather than opt-out. Its counterpart in the wordpress-develop repo makes WordPress explicitly set the ALLOW_EXPERIMENT_REREGISTRATION to false: WordPress/wordpress-develop#4121
adamziel
added
[Type] Bug
|
Size Change: +559 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 7e9d944. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4251868954
|
gziolo
approved these changes
Feb 28, 2023
Member
There was a problem hiding this comment.
I'm not the best suited to review it as I miss full context, but it seem to resolve the issue. It makes the flag optional only where necessary which is always good.
Unrelated to this PR, I saw recently that process.env is no longer integrated with webpack 5 so maybe it isn't the most future proof method but it will work for WordPress Core that we care about.
Member
|
Totally optional as I had this afterthought after clicking |
Contributor
Author
Contributor
Author
I tried to come up with a better name, but couldn't. This API is now for private APIs, not experiments, so it would have to be |
Member
Maybe we need |
Contributor
Author
|
Ooh i like it @gziolo, let's go for that one |
Member
|
I'm curious if one day we could simplify it to |
gziolo
approved these changes
Feb 28, 2023
Member
gziolo
left a comment
gziolo
left a comment
There was a problem hiding this comment.
Still good to go and I like the magic global more now.
Further ideas are better suited for a different PR as it might have implications on TS and ESLint configuration.
adamziel
changed the title
adamziel
deleted the
make-allow-reregistration-opt-in-rather-than-opt-outw
branch
adamziel
changed the title
3 tasks
Contributor
|
Do we still need this? This basically prevents plugins relying on Gutenberg active to catch real bugs soon (before removing the Gutenberg dependency) |
Member
|
We could redo it in a way where it works smoothly with the Gutenberg plugin. Otherwise devs hack the registry to use private APIs in the plugins that depend on Gutenberg. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Gutenberg introduced a system of sharing private APIs in #46131. One of the safeguards is a check preventing the same module from opting-in twice so that contributors cannot easily gain access by pretending to be a core module.
That safeguard is only meant for WordPress core and not for the released @WordPress packages. However, right now it is opt-out and must be explicitly disabled by developers wanting to install the @WordPress packages. Let's make it opt-out instead.
In other words:
Or:
The corresponding PR in
wordpress-developrepo makes WordPress explicitly set theIS_WORDPRESS_COREto true:WordPress/wordpress-develop#4121
Test plan
packages/data/src/js/private-apis.jsto the code snippet below