Metaboxes registered after initial initialisation are not saved

[lgladdy]

Description

Somewhere around WordPress 5.8, Gutenberg stopped saving Advanced Custom Fields data for metaboxes which are added to pages after the page has been initialised.

This is because the hasMetaBoxes constant is only checked on init:

gutenberg/packages/edit-post/src/store/actions.js

Line 570 in 46179e2

const hasMetaBoxes = select.hasMetaBoxes();

This means for any metaboxes added to the page after init (in ACF's case, this happens if a field group is only assigned to a specific category, and the user changes that page to that category), the check for shouldTriggerMetaboxesSave is always false:

gutenberg/packages/edit-post/src/store/actions.js

Line 585 in 46179e2

const shouldTriggerMetaboxesSave =

ACF does dispatch setAvailableMetaBoxesPerLocation when it adds a new metabox after init, but that doesn't trigger the update for hasMetaBoxes to allow them to save.

We don't think this is an issue we can solve our side, short of registering a "hidden" meta box that is always present to trigger the save - we're also happy to raise a PR to help fix this in Gutenberg too, but would love some advice and direction on the best place to add the logic to fix this.

Thanks!

Step-by-step reproduction instructions

1. Add a metabox to a Gutenberg page after it's been initialised.
2. Save the page - because hasMetaBoxes was empty on init, the shouldTriggerMetaboxesSave check will return false when it should be aware new metaboxes have been added and save.

I threw together a minimum viable demo of the issue here: https://github.com/lgladdy/gutenberg-issue-44716-demo/blob/main/after-init.js

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

No

[Mamaduka]

Hi, @lgladdy

Can you provide a minimal code example to reproduce the issue without ACF?

[lgladdy]

@Mamaduka Sure! I've thrown together a quick plugin that shows the issue via a simple JS loaded into the block editor, https://github.com/lgladdy/gutenberg-issue-44716-demo

If you toggle between the window.setTimeout which adds the meta box after 2 seconds vs calling addMetabox() immediately on .ready() you can see the issue.

Updating a post where the meta box was added immediately, the editor will save metaboxes with a call to post.php after the main post update. For the meta box added after 2 seconds, that call to post.php never fires, because of the code I pointed to in the original issue post.

[lgladdy]

Oops, sorry - repo is now public.

The main code that needs to run to demo this is here: https://github.com/lgladdy/gutenberg-issue-44716-demo/blob/main/after-init.js - if you'd rather run it in some other way than a plugin!

[Mamaduka]

Thanks for the reproduction code, @lgladdy!

I think the original behavior changed after the changes from #30617 landed in WP 5.8.x.

@jsnajdr, I know you're working on an improved flow for saving meta boxes as part of #44971. Would it be possible to account for lazy loaded meta boxes in that flow?

[jsnajdr]

I think the original behavior changed after the changes from #30617 landed in WP 5.8.x.

Yes, the buggy behavior is a combination of #15041, which moved the hasMetaBoxes() check out of the subscribe listener, and of #30617 which stopped re-creating the subscription on every setAvailableMetaBoxesPerLocation call, together with a fresh hasMetaBoxes() call.

ACF does dispatch setAvailableMetaBoxesPerLocation when it adds a new metabox after init

@lgladdy This is probably not 100% reliable because setAvailableMetaBoxesPerLocation overwrites the data that were set by the previous call. If any metaboxes were created during page initialization, possibly by another plugin, calling setAvailableMetaBoxesPerLocation just with the one new dynamically added metabox will overwrite them.

Maybe we could change setAvailableMetaBoxesPerLocation to merge the new data with the old? The data structure is very amenable to merging, because all items have unique IDs.

I'm fixing the hasMetaBoxes() call in #45145, could you please verify that it indeed fixes the ACF issue?

[lgladdy]

Just to add my +1 that this resolves the issue in ACF. Thanks @jsnajdr! 🥳

[jsnajdr]

@lgladdy I'm improving the setAvailableMetaBoxesPerLocation action in #45156, so that, as described above, existing metaboxes are not lost when new ones are registered at runtime. Please try it out!