Skip to content
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

Opt-out from re-registration of private APIs. #4121

Closed

Conversation

adamziel
Copy link
Contributor

@adamziel adamziel commented Feb 23, 2023

Description

Gutenberg introduced a system of sharing private APIs in
WordPress/gutenberg#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 opts-out from that check in WordPress core by setting
the IS_WORDPRESS_CORE to true. Once it's merged,
the Gutenberg plugin should be adjusted to use false as the
default value.

In other words:

  • Before this PR and Gutenberg#48352, double opt-in safeguard is enabled by default
  • After this PR and Gutenberg#48352, double opt-in safeguard is disabled by default AND WordPress core explicitly enables it

Test plan:

Merging this PR will have no immediate effect:

  • At the moment, WordPress core contains a Gutenberg version where re-registration is not allowed.
  • Merging this PR on its own will not change anything in that Gutenberg version.
  • The related Gutenberg PR allows the re-registration by default, but will only affect WordPress core once it is backported to wordpress-develop repo.

I recommend:

  • Merging this PR now to make sure it's not overlooked later on
  • Not backporting the Gutenberg PR for 6.2

This way, the check will be enabled by default in 6.2 and enabled explicitly in 6.3.

Trac ticket: https://core.trac.wordpress.org/ticket/57795

Gutenberg introduced a system of sharing private APIs in
WordPress/gutenberg#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 opts-out from that check in WordPress core by setting
the ALLOW_EXPERIMENT_REREGISTRATION to false. Once it's merged,
the Gutenberg plugin should be adjusted to use `true` as the
default value.
@adamziel
Copy link
Contributor Author

adamziel added a commit to WordPress/gutenberg that referenced this pull request Feb 23, 2023
…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
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it looks like we can safely land it at any time. It won't take effect until WordPress packages are synced with the related changes. The change impacts only the build process.

adamziel added a commit to WordPress/gutenberg that referenced this pull request Feb 28, 2023
…opt-out via IS_WORDPRESS_CORE (#48352)

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:

* If we’re in WP core – prevent opting-in to private API with the same package name twice
* If we’re not in WP core – don’t prevent it 

Or:

* Before this commit, double opt-in safeguard is enabled by default
* After this commit, double opt-in safeguard is disabled by default AND WordPress core [explicitly enables it](WordPress/wordpress-develop#4121)

The corresponding PR in `wordpress-develop` repo makes WordPress explicitly set the `IS_WORDPRESS_CORE` to true:

WordPress/wordpress-develop#4121
@adamziel
Copy link
Contributor Author

@hellofromtonya @peterwilsoncc let me know if you think this is suitable for merging, and if yes then I could make it my first commit.

@hellofromtonya
Copy link
Contributor

@adamziel I marked the ticket for commit and assigned the commit to you. Go ahead with your first commit 🎉

@adamziel
Copy link
Contributor Author

Committed in https://core.trac.wordpress.org/changeset/55512/trunk – thank you everyone! 🎉

@adamziel adamziel closed this Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants