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

Add a “waitForTransition” data API #41169

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from

Conversation

adamsilverstein
Copy link
Member

What?

Add a new "subscribeOnChange" API.

Why?

Based on feedback from @youknowriad - #17632 (comment) this API helps address a common request for a "Post Published Hook". (or really a "post anything hook").

How?

Developers can now call subscribeOnChange with a callback function and a test they want to trigger "After", for example saving a post would be monitored with a callback using isSavingPost.

Testing Instructions

  1. Open or create a test post.
  2. Paste the following snippet into the console:
    wp.data.subscribeOnChange( () => { console.log( "Post saved." ); }, () => wp.data.select( 'core/editor' ).isSavingPost() );
  3. Save the post. Note that "Post Saved" is triggered once (and only once) after the post save completes

Open questions

  • Does always watching for true/false make sense or should the API accept values to test for start/end
  • Have I exposed and documented the API correctly? The data package structure is complex and I'm not sure I did this right
  • Currently the function fires only once, should it instead "reset", waiting for false again first (then true and false again before triggering)? As is developers would need to re-add the listener each time
  • Should the function return an unsubscribe function (similar to subscribe) so developers can unhook their listeners?

@Mamaduka Mamaduka added [Package] Data /packages/data [Type] New API New API to be used by plugin developers or package users. labels May 20, 2022
@Mamaduka Mamaduka requested a review from jsnajdr May 20, 2022 03:50
@gaambo
Copy link
Contributor

gaambo commented Nov 16, 2022

Hi, is this still being considered or are there any things to help with?
Such a hook would be great for non-React JavaScript to run certain things (eg metaboxes or other customized editor implementations).

@adamsilverstein
Copy link
Member Author

@gaambo I haven't had a chance to revisit, see the last comment on the thread (#41169 (comment)) for what needs work next here if you want to give it a try.

* Rename helper from subscribeOnChange to waitForTransition
* Move helper to factory.js and rename -> utils.js + update references
* Update readme example
* Handle cancelled promise, unsubscribing
@adamsilverstein
Copy link
Member Author

Hi, is this still being considered or are there any things to help with? Such a hook would be great for non-React JavaScript to run certain things (eg metaboxes or other customized editor implementations).

Hey @gaambo thanks for the offer of help! One way to help would be to test the approach to see if it works for you; another would be to add some tests that covered this function. Do you want to try to work on one of those?

@gaambo
Copy link
Contributor

gaambo commented Aug 21, 2023

Hey @gaambo thanks for the offer of help! One way to help would be to test the approach to see if it works for you; another would be to add some tests that covered this function. Do you want to try to work on one of those?

Hi, sorry I never got to do more with this issue. I tried a few things, but that stuff was above my head 😅

In the project I initially needed this I solved it like this:

window.wp.data.subscribe(() => {
    const isSavingMetaBoxes = window.wp.data
        .select("core/edit-post")
        .isSavingMetaBoxes();

    const isDoneSaving = wasSaving && !isSavingMetaBoxes;

    if (isSavingMetaBoxes && !hasChangedInputs) {
        // Do something while/before saving
    }
    if (isDoneSaving) {
        // Do something after saving
    }

    wasSaving = isSavingMetaBoxes;
});

Which comes down to

window.wp.data.subscribe(() => {
    const isSavingMetaBoxes = window.wp.data
        .select("core/edit-post")
        .isSavingMetaBoxes();
});

And I'm not sure if the solution after this PR is so much better/shorter.

waitForTransition(  () => wp.data.select( 'core/editor' ).isSavingPost() ).then( () => {
  // Do something when the post is done saving.
  console.log( 'Post saved!' );
} );

I think my main concern with my current implementation was, that I'm afraid the subscribe callback would be called on each and every (=any) change in the datastore. But it seems the implementation in PR also has this "problem"?

I see in this PR, it unsubscribes after triggering = it will only get triggered once. In my case I needed it to run on every save again.

packages/data/src/index.js Outdated Show resolved Hide resolved
@jsnajdr
Copy link
Member

jsnajdr commented Aug 21, 2023

I see in this PR, it unsubscribes after triggering = it will only get triggered once. In my case I needed it to run on every save again.

In that case you can run waitForTransition in a loop:

while ( true ) {
  await waitForTransition( ... );
  console.log( 'Post saved!' )
}

The downside of this is that transitions that happen outside the waitForTransition call will not be catched. For example, when there is a transition that happens during the console.log call, or the user code that would replace it in real-live code.

We'd need to start using Observables to create an "async stream of transition events".

I'm afraid the subscribe callback would be called on each and every (=any) change in the datastore.

Yes, this will be happening and it's not a problem. There's no other way to detect "relevant changes" other than inspecting "all changes".

Copy link

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: adamsilverstein <[email protected]>
Co-authored-by: jsnajdr <[email protected]>
Co-authored-by: gaambo <[email protected]>

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

@adamsilverstein adamsilverstein changed the title Add a “subscribeOnChange” API Add a “ waitForTransition” data API Jan 13, 2025
@adamsilverstein adamsilverstein requested review from jsnajdr and gaambo and removed request for gaambo January 13, 2025 22:07
@adamsilverstein adamsilverstein changed the title Add a “ waitForTransition” data API Add a “waitForTransition” data API Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Data /packages/data [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants