Site Editor: Export includes layout definitions from lib/theme.json

[jasmussen]

Extracting from WordPress/create-block-theme#324 (comment), and related to #49914 with further context in #49914 (comment).

When you build a theme in the site editor, then export it using the ellipsis > Export option, the theme.json file includes settings.layout.definitions in the export.

These definitions should only ever be in the core theme.json, as they are used to build the core styles for each layout type. They should never be in theme files, as they are now. #49914 is an example of what can happen when they are included, that is: any bugs present in the layout definitions when the theme was exported get permanently applied to the theme, since those definitions override the core definitions. Additional results could be that any new features applied to layout in newer versions of theme.json will invisibly fail or just not work.

Steps to reproduce:

• Activate a block theme like TT3
• Go to global styles and make a change to the global layout like editing the default content width
• Use the site editor to export the theme

Observe how it includes all definitions copied over from lib/theme.json.

This is urgently in need of fixing, lest every theme created until fixed, will have hard-to-debug future layout issues.

[jasmussen]

Question: Do these definitions need to be part of the core theme.json at all? Since they relate to core layout and should never be customized or exported, shouldn't they live elsewhere?

[pbking]

Note that this seems to be addressed in the current branch of WordPress 6.3 (regardless of Gutenberg plugin usage). A quick scan wasn't able to show me what changed in core to make this work as expected.

[andrewserong]

Thanks for opening up this issue and for the detailed description of the problem! I believe this was a regression introduced in #48070 — it appears that change accidentally included all of the layout object when making changes to contentSize or wideSize, which rolled the layout definitions into the user data unexpectedly.

I have a fix open in #50268 which strips the layout.definitions object from the user data when it gets saved. With that fix in place, the definitions object should no longer be included in exports, because it'll no longer be saved to user data. For any sites that currently have the definitions object within the user data, (if that PR gets merged), then the next time they go to change contentSize or wideSize, the definitions object will be flushed from the user data. For anyone working on test themes, you'll want to make a change to either of those value before re-exporting.

Question: Do these definitions need to be part of the core theme.json at all? Since they relate to core layout and should never be customized or exported, shouldn't they live elsewhere?

That's a good question. When they were introduced into the core theme.json, the assumption is that they wouldn't ever leak into theme or user data. I remember there was also the idea that potentially we might want themes to one day be able to customize how layout definitions work — but since then, I don't think anyone's proposed that as being a good idea. The layout definitions certainly could live elsewhere, but it might be a fair bit of work to move them at this stage. It would be a good thing to explore in the longer-term where best to keep the central definitions, though.

As for next steps, assuming #50268 is a good enough fix for now, that should solve the immediate issue. If we need to harden it further, then a next step could be to see if we can update the getters for merged theme.json data so that the layout definitions are always pulled from the core data, and not theme or user.