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

Create a catalog list of private APIs #66558

Merged
merged 4 commits into from
Dec 13, 2024
Merged

Create a catalog list of private APIs #66558

merged 4 commits into from
Dec 13, 2024

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Oct 29, 2024

This PR adds a list of all private APIs in Gutenberg packages. Next steps:

  • make sure that the list is complete
  • assess the level of stability of each API: how long it's been around, where is it used
  • come up with a stabilization plan for each API

@jsnajdr jsnajdr added the [Type] Developer Documentation Documentation for developers label Oct 29, 2024
@jsnajdr jsnajdr self-assigned this Oct 29, 2024
Copy link

github-actions bot commented Oct 29, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jsnajdr <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: youknowriad <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

I think this might make sense in a tracking issue, rather than in a document. The tracking issue will allow people to assign themselves and add context, effortlessly, without having to commit to the repo.

WDYT?

Comment on lines 3 to 7
Gutenberg packages currently export 237 private APIs:
- 42 store actions
- 57 store selectors
- 66 React components
- 30 React hooks
Copy link
Member

Choose a reason for hiding this comment

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

Do the numbers have any specific utility? I'd refrain from adding those numbers because it will be very easy for them to get obsolete.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand this feedback. Is is about the usefulness of the numbers (I think they are very interesting!) or about them being placed in the document, rather than is some (interesting) GitHub comment?

Copy link
Member

Choose a reason for hiding this comment

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

The numbers can be useful; we agree there. However, it will be difficult/extra effort to keep them up to date, which invalidates their usefulness IMHO.

@@ -0,0 +1,348 @@
# Gutenberg Private APIs
Copy link
Member

Choose a reason for hiding this comment

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

Might make sense to add context to the document, explaining why it exists and why we need to catalog those private APIs.

@jsnajdr
Copy link
Member Author

jsnajdr commented Oct 29, 2024

I think this might make sense in a tracking issue, rather than in a document.

My thinking is that this is something that is going to exist forever, and also it's a form of documentation. So it should be a document in the repo. Issues are transient and they are supposed to be eventually closed.

An inspiration is the TC39 repo where the info about proposals and theirs status is also committed in version control, as .md documents.

@tyxla
Copy link
Member

tyxla commented Oct 29, 2024

I think this might make sense in a tracking issue, rather than in a document.

My thinking is that this is something that is going to exist forever, and also it's a form of documentation. So it should be a document in the repo. Issues are transient and they are supposed to be eventually closed.

An inspiration is the TC39 repo where the info about proposals and theirs status is also committed in version control, as .md documents.

That makes sense 👍 I like the TC39 table view of different stage proposals.

@youknowriad
Copy link
Contributor

While I encourage this initiative, it's important to note that not all private APIs are meant to be "stabilized". Private APIs are in fact stable APIs but they are internal to Core. While we'd probably want to open some of them as public APIs, it's not their purpose to be used as "experimental APIs"

@jsnajdr jsnajdr force-pushed the add/private-apis-catalog branch from bb9e0e8 to b0d46bc Compare December 5, 2024 12:51
Copy link

github-actions bot commented Dec 5, 2024

Flaky tests detected in d7a59ed.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12256321622
📝 Reported issues:

@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 5, 2024

I resolved the review feedback and incorporated a few changes that happened recently. I believe this PR should be mergeable now.

I'd refrain from adding those numbers because it will be very easy for them to get obsolete.

@tyxla I removed the stats from the document, keeping only the list of APIs.

Might make sense to add context to the document, explaining why it exists and why we need to catalog those private APIs.

Added some motivational text at the beginning of the document.

it's important to note that not all private APIs are meant to be "stabilized". Private APIs are in fact stable APIs but they are internal to Core.

@youknowriad That's fine, my takeaway is that we should consistently call them "private" and avoid using the term "experimental" or "unstable".

Which APIs do you think currently are of this type? Private by design? Let's mark them as such in this document.

@youknowriad
Copy link
Contributor

Which APIs do you think currently are of this type? Private by design? Let's mark them as such in this document.

Hard to tell, there's so much but I think the majority is like that.

@youknowriad
Copy link
Contributor

Part of me wonders if having this catalog would actually encourage users to use these (usurp some unloaded package to get access to these)

@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 5, 2024

Part of me wonders if having this catalog would actually encourage users to use these

That's not necessarily a bad thing. If these APIs are valuable to plugin authors, and they help them build something valuable, then why are they private? The question is whether we have a way to learn about these usages.

Hard to tell, there's so much but I think the majority is like that.

Let's start flagging specific ones.

@youknowriad
Copy link
Contributor

That's not necessarily a bad thing. If these APIs are valuable to plugin authors, and they help them build something valuable, then why are they private? The question is whether we have a way to learn about these usages.

That is definitely a bad thing for me. Plugin authors should absolutely not use these APIs. This is the reason we introduce the private APIs concept in the first place.

I think you're conflating "experimental APIs" which we don't have any concept of at the moment and for which we want to discovery with private APIs which we do have a concept for today and we want them to stay private.

@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 6, 2024

That is definitely a bad thing for me. Plugin authors should absolutely not use these APIs.

My issue with this is that the private-ness of many of these APIs is going to be hard to defend and self-contradictory. Especially when they are long-term stable.

If a plugin goes through the trouble to access a stable private API, they probably do it because they are implementing something valuable with it. The API is interesting and useful for plugins. Then, if it's stable and useful, why are we keeping it private? The Gutenberg plugin obviously also finds that particular private API useful and valuable, because it uses it to implement things. Core blocks use private APIs from block-editor all the time. So does Post Editor or Site Editor. Then why cannot a custom block or a custom editor integration use it, too?

For example, #67209 discusses the BlockCanvas.contentRef prop. BlockCanvas is a public component in block-editor, a module which "allows you to create and use standalone block editors". But my standalone block editor cannot use contentRef, because it's only in the secret ExperimentalBlockCanvas version of the component. Even though it's a stable zero-risk prop. And when we tell plugin authors to not use contentRef and implement their feature some other way, then why does VisualEditor need contentRef to implement 7 features? It should lead by example and re-implement them without contentRef. Maybe we would discover that we cannot really live without the prop and that's it's a good API after all.

I think you're conflating "experimental APIs" which we don't have any concept of at the moment and for which we want to discovery with private APIs which we do have a concept for today and we want them to stay private.

Yes, I am conflating them because at this moment they are the same thing. There was a proposal in #47785 to also have "plugin-only" APIs, but it was never implemented. It seems that we are going to implement that only now.

Also I don't believe the boundary between private and experimental is going to be that strict. I think that will show when we start discussing individual endpoints one by one.

@youknowriad
Copy link
Contributor

Especially when they are long-term stable.

For me writing a stable internal API and writing a stable public API is two completely different things with different tradeoffs.

A private API that is stable for users doesn't mean it should be public. Think of it as private property in a class or a function that is not exported in a JS module.

It's not because this function or property hasn't changed for years that it's meant to be used by plugins.

If a plugin goes through the trouble to access a stable private API, they probably do it because they are implementing something valuable with it. The API is interesting and useful for plugins.

I have no doubt that some of these APIs are valuable for plugins, but instead of just highlighting the wrong thing to do, we should be encouraging the plugins to require public APIs. At that moment we can discuss whether to build a public API, think of whether the function/class/component that was private has the correct tradeoffs in place to be made stable.

We should find ways to make it even more strict to access these APIs and not the opposite.

ContentRef #67209

This is actually a good example of what I'm trying to say. This API should never be public, the plugin author shared its use-case and it's not something that should be addressed by making contentRef public, in most situations, the public API needed (in this case, a labels/messages API) is very different from the way plugin authors are trying to use these private APIs.

@jsnajdr jsnajdr force-pushed the add/private-apis-catalog branch from b0d46bc to 302e9d4 Compare December 9, 2024 13:00
@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 9, 2024

We should find ways to make it even more strict to access these APIs and not the opposite.

I totally agree with this and I plan to propose to lock the truly private APIs even more, using some private symbol as key or by a key uniquely generated on every build.

But to make this work we need to address the private APIs that are currently used by plugins. Because the workarounds like when WooCommerce pretends to be @wordpress/edit-site to get access to editor private APIs will stop working.

I see a pattern where anyone who wants to build a custom block editor is using some private API to do that. The P2 editor, MailPoet, WooCommerce product editor. Even the Drupal integration does something similar to <BlockCanvas contentRef>: it searches for the content element using a MutationObserver: https://git.drupalcode.org/project/gutenberg/-/blob/3.0.x/js/editor-observers.js

That shows that the block-editor and editor packages are not really ready to be independent packages that you could build custom editor with.

This API should never be public, the plugin author shared its use-case and it's not something that should be addressed by making contentRef public, in most situations, the public API needed (in this case, a labels/messages API) is very different from the way plugin authors are trying to use these private APIs.

What strikes me as odd is how many features in Gutenberg rely on contentRef:

  • the typewriter experience where editor scrolls to keep the caret on the same horizontal line
  • clicking at the bottom of the post inserts a new block
  • scroll events on most popovers are forwarded to the editor iframe so that the popover is "scroll-transparent"
  • clicking inside the editor in locked mode selects the nearest editable block
  • double clicking in zoom mode leaves the zoom mode
  • double clicking on non-editable template or block shows a notification
  • resizable editor watches the height of the editor iframe
  • find the block that should be a target of a drag&drop operation

In short, there is a long list of functionality that is interested in looking directly at the DOM elements in the block editor, measure them or attach event handlers. It doesn't sound very likely that it could be all addressed with specialized public API.

But I believe we're already very off-topic in the context of this PR 🙂 Can I merge the current list of private APIs or is there any blocker?

@youknowriad
Copy link
Contributor

For me, I don't think we should merge this PR as is as it frames things as APIs that belong to different stages of stabilisation. This only makes the problem worse as plugin authors will expect these APIs to move to another stage while they shouldn't.

@jsnajdr jsnajdr force-pushed the add/private-apis-catalog branch from 302e9d4 to d7a59ed Compare December 10, 2024 12:41
@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 10, 2024

it frames things as APIs that belong to different stages of stabilisation.

I rewrote the introductory paragraphs to point out that many of them are permanently private. Let me know if it looks better now. And I'm looking forward to some actionable feedback in case it still doesn't.

My goal is to present a picture of the current status quo, without creating any expectations or making promises about what will happen in the future.

@youknowriad
Copy link
Contributor

It's better but personally I think we should just not do this at all. (just my opinion)

It makes it sound like the number of private APIs is something that we should be careful about? I think that's wrong, no counts how many private functions or classes they have in their code base. Do we list all private PHP class properties, functions in WordPress docs?

Whether we like or not, this highlights these APIs more and plugin authors will use this document as a way to discover these APIs, which we don't want to.

@youknowriad
Copy link
Contributor

To clarify my thoughts, I think a catalog like proposed here is very valuable for actual "experimental" APIs. But as of today, we don't have any (I actually think we have two, the ones hidden behind IS_GUTENBERG_PLUGIN to register actions and fields).

@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 10, 2024

Do we list all private PHP class properties, functions in WordPress docs?

Yes, we do. The private functions with a _ prefix are documented (random example) and private class methods and fields are documented, too (another random example). The docs have a notice on top saying "not intended for use by plugin or theme developers, only in other core functions". Apparently the docs are also for core developers themselves, not only for plugin and theme authors.

It makes it sound like the number of private APIs is something that we should be careful about?

I disagree that the document makes any judgments about the APIs. It just lists them. Someone might consider it fine, someone else might find it embarrassing or whatever, but that's always their judgment, it's not in the text.

Whether we like or not, this highlights these APIs more and plugin authors will use this document as a way to discover these APIs, which we don't want to.

Is this really how plugin authors discover Gutenberg APIs, by reading their documentation? I don't think that our docs are that good. From what I've seen in 3rd party plugin code, their authors almost certainly must have read the actual Gutenberg sources and draw inspiration and examples from there. And often they are already very well aware about the private/experimental APIs.

I disagree that merely describing something can possibly make things worse.

To clarify my thoughts, I think a catalog like proposed here is very valuable for actual "experimental" APIs.

I'm all for that, but currently none of the APIs are designated as experimental. And even the private API catalog is useful for Gutenberg contributors: there are over 200 private APIs now and are widely used (I see 560 unlock() calls in the codebase). And block-editor has actually more private APIs than the public stable ones, so why should they be kept secret and undocumented?

@youknowriad
Copy link
Contributor

youknowriad commented Dec 10, 2024

If we really want to ship this I would remove these two sentences:

The purpose of this document is to present a picture of how many private APIs we have, and provide some basic information about what purpose they serve and whether they have a roadmap to become public. Some of them will eventually be reclassified as "experimental" and exposed using a different mechanism.

Many are supposed to be private permanently, and others have or will have a roadmap to become stable and public.


The differences between the APIs exposed here and the ones you mention as documented is the fact that both of these two random examples are actually publicly accessible APIs (regardless of whether they're marked as private). In the case of our private APIs in Gutenberg, they're not just marked as private, we do our best to prevent access as well. I don't see personally any difference between these and any random function or component that is not exported from the packages, so why are we listing just the private APIs that "leak" through the packages.

@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 11, 2024

both of these two random examples are actually publicly accessible APIs (regardless of whether they're marked as private).

One of these examples is a private class method, with a real private keyword, not just some @private doc comment. WordPress Core really documents almost everything, even the internals.

If we really want to ship this I would remove these two sentences:

Because they mention that some private APIs could become eventually public, and therefore they set expectations and encourage their use? That's fair.

I'm also OK with posting this info in a GitHub issue, I just want to have the list at some place that has a public URL so that folks can refer to it. But I guess that having an issue suggests that private APIs are a "problem to be solved", something that we want to avoid. A .md document in the repo is more neutral, more like "this is the status-quo".

@youknowriad
Copy link
Contributor

Because they mention that some private APIs could become eventually public, and therefore they set expectations and encourage their use? That's fair.

Yes, that's my main concern with these sentences.

@jsnajdr jsnajdr force-pushed the add/private-apis-catalog branch from d7a59ed to 89f5906 Compare December 12, 2024 11:30
@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 12, 2024

Yes, that's my main concern with these sentences.

I rewrote the introductory text trying to convey the following two ideas:

  • the document describes our private APIs. It's the status quo, they are private, and we're not making any guesses about how they will be in the future.
  • these private APIs are used to build the "real" Gutenberg editor apps out of the low-level packages, that's their purpose.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Looks good.

How are we planning to keep this list up to date.

@jsnajdr
Copy link
Member Author

jsnajdr commented Dec 13, 2024

How are we planning to keep this list up to date.

It could be eventually automated, in a very similar way how we now generate docs for the public APIs.

How I created this list in the first place: I instrumented the lock calls by adding a console.log statement logging the fields of the object that lock is locking, and then copied the console output to the .md document. So, already semi-automated.

@jsnajdr jsnajdr merged commit 20f4174 into trunk Dec 13, 2024
63 checks passed
@jsnajdr jsnajdr deleted the add/private-apis-catalog branch December 13, 2024 10:26
@github-actions github-actions bot added this to the Gutenberg 20.0 milestone Dec 13, 2024
yogeshbhutkar pushed a commit to yogeshbhutkar/gutenberg that referenced this pull request Dec 18, 2024
* Create a catalog list of private APIs

* Document some private components

* Rewrite the introduction

* Rewrite the introduction again
yogeshbhutkar pushed a commit to yogeshbhutkar/gutenberg that referenced this pull request Dec 18, 2024
* Create a catalog list of private APIs

* Document some private components

* Rewrite the introduction

* Rewrite the introduction again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants