-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Private APIs: remove re-registration check #73610
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
Conversation
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @copilot. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
|
This makes sense to me. A core package being bundled multiple times is definitely a valid use-case that we need more and more. It does make it easier for third-parties to opt-in into these private APIs but I have two thoughts here
|
|
Size Change: -47 B (0%) Total Size: 2.54 MB
ℹ️ View Unchanged
|
|
@copilot looks like that failing unit test can be removed, can you do that? |
|
Flaky tests detected in f4bb8ed. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19734941612
|
|
For context, the current state (allow re-registration in Gutenberg but do not allow it in core) has proven an issue. It hides the issue until the worst-possible moment (package updates in core). This caused:
#73555 analyzes the usage of private APIs in DataViews/DataForm. In conversations with @mirka some of them are not ready to be made public (the validation components) because they're built on older version of low-level primitives — we are working on newer versions at |
|
I think we should backport this to the last GB RC, so we can sync packages in core from 22.2 onwards instead of having to wait another two weeks. |
Co-authored-by: ellatrix <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: oandregal <[email protected]>
|
I just cherry-picked this PR to the release/22.2 branch to get it included in the next release: 68cacfb |
What?
Remove the re-registration check from the private APIs package. This allows packages to opt-in to private APIs multiple times.
Why?
Some of our packages like data views and global styles UI are bundled in other packages multiple times, which causes the private APIs registration to complain about multiple opt-ins, except in Gutenberg itself. This means we have a behavior difference between Gutenberg and Core, which is preventing package syncs from happening because the editors crash.
First of all, the behavior should be the same for core and Gutenberg.
Secondly, the way that some modules like data views and global styles UI are being developed makes it hard to enable the re-registration check in the GB repo.
The fact that people have to go through a lot of effort to use private APIs (install private api packages, opt-in string, unlock...) should be enough to communicate that these APIs are private and there's to backwards compatibility. There's quite a few cases where people create workaround even around the re-registration, e.g. by npm package patches, which makes everything even more fragile. When packages are consumed through npm, there's also less concern because of the versioning and people should be able to use the private APIs if they really wanted to.
So I propose that we simply remove the check. 🙂
How?
Removes it.
Testing Instructions
Tests should pass.
Testing Instructions for Keyboard
Screenshots or screencast