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

Private APIs: Improve the unlocking mechanism when paired with data stores #47830

Closed
gziolo opened this issue Feb 7, 2023 · 3 comments
Closed
Labels
Developer Experience Ideas about improving block and theme developer experience [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.

Comments

@gziolo
Copy link
Member

gziolo commented Feb 7, 2023

What problem does this address?

Part of #47786.

It's based on the observation I shared after studying the refactoring applied by @talldan in #47375 to use hide the experimental APIs with the locking/unlocking system. The comment is available at #47375 (comment):

One potential drawback to calling unlock on the entire data store exists. After a couple of refactorings, it will be impossible to tell which selectors/actions are experimental. In the case of promoting an experiment to a stable API, the unlock call will stay in place as there won't be any incentive to refactor the code. In addition, it's going to be difficult to tell whether this proxy can be safely removed without running the code, because several more than one selector/action could be experimental.

What is your proposed solution?

Two solutions were proposed by @adamziel in #47375 (comment):

  1. Don't expose stable selectors on the unlocked store. Drawback: Now you need two useSelect() calls.
  2. Use a prefix like __internal to make the difference apparent. Drawback: There's still a prefix.

I don't like either of these solutions. This issue could use a separate discussion thread.

@talldan commented on the idea (1) in #47375 (comment) with:

I don't think it'd require two useSelect calls, it'd look a bit like this:

const { public, private } = useSelect( select ) => {
  const { publicSelector } = select( myStore );
  const { privateSelector } = unlock( select( myStore ) );
  return { public: publicSelector, private: privateSelector };
} );

I responed to the idea (2) in #47375 (comment) with:

It might help if the prefix was automatically prepended by the lock call. Still, not ideal 😄

@gziolo gziolo added [Type] Code Quality Issues or PRs that relate to code quality Developer Experience Ideas about improving block and theme developer experience labels Feb 7, 2023
@adamziel
Copy link
Contributor

adamziel commented Feb 7, 2023

cc @youknowriad @jsnajdr @talldan

@kathrynwp kathrynwp added the [Type] Enhancement A suggestion for improvement. label Feb 7, 2023
@youknowriad
Copy link
Contributor

No strong opinion for me. I'm not sure it's very important personally to distinguish between locked and unlocked selectors in core components but regardless, @talldan's idea is fine by me.

@gziolo gziolo added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Feb 13, 2023
@gziolo
Copy link
Member Author

gziolo commented Feb 13, 2023

Actually, my only concern was that we would end up with lingering unlock calls on stores, but it might be partially covered with the following line:

if ( ! ( __private in object ) ) {
throw new Error(
'Cannot unlock an object that was not locked before. '
);
}

We will see how it looks in practice.

@gziolo gziolo closed this as completed Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Code Quality Issues or PRs that relate to code quality [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants