Extra action triggered when saving posts

[youknowriad]

There's a redux action that is triggered on the "core" store at the end of the "post saving" but this is not really an action (it's the actual post object) and shouldn't be triggered. (see the "post'" action in the screenshot)

cc @epiqueras

[epiqueras]

This is because we had to make the action return the updated record in #17030, because that was the previous behavior and plugins relied on it.

[youknowriad]

Should we revisit this to fix that unintended behavior. It will probably mean a "Dev Note" but it's better than keeping a hidden bug. Thoughts?

cc @nerrad @aduth

[nerrad]

Should we revisit this to fix that unintended behavior

To clarify, the behaviour where the action is returning the updated record is considered a bug?

If so, there's some discussion in this pull about this and it'd be good if any #dev-note includes the best practice way to handle the scenario described in that pull.

[youknowriad]

To clarify, the behaviour where the action is returning the updated record is considered a bug?

What's considered a bug is the fact that that record is considered as a redux action and is dispatched automatically if it contains a "type" key which is the case for post entities.

The fix could be to "just" avoid considering the returned objects as actions but If I understood properly, we might not be able to do it without breaking changes?

[epiqueras]

Both are breaking changes.

• A: Not considering the returned objects as actions is a breaking change and we would need to refactor our own actions.
• B: Not returning the post here is a breaking change.

I am leaning more towards A here, even if that requires more refactoring work on our part.

If we go with B, getting the post would involve subscribing to the store to figure out when the action resolved and the post was updated. This makes it a pain to use in promise driven workflows and people will just resort to using the API directly.

Also, it might be just me, but the fact that you can either yield or return an action doesn't sit right with me, and it's making our JSDOCs inconsistent, so we would also kill that bird with this stone.

[aduth]

Previously: #14711 (comment), #14830

It never really sat well with me that we would resolve dispatch with some "resulting value" (a post object). That said, for the use-cases presented, it didn't seem at the time that there was much of an alternative.

If we go the route that we stop treating the return from a generator action as an action, what impact does this have on synchronous actions? What is the resolved value from dispatch for those actions? The action object? undefined ?

I agree with @epiqueras that it's odd we support action dispatching from generators by both yield and return statement. I think there's no reason we couldn't just enforce that only yield is considered an action. Technically it's a breaking change, but I'm not sure we ever really documented this as an expected behavior?

[epiqueras]

If we go the route that we stop treating the return from a generator action as an action, what impact does this have on synchronous actions? What is the resolved value from dispatch for those actions? The action object? undefined ?

I'd say undefined.

[youknowriad]

This seems fixed.