-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix error in site-editor when presets are null #30144
Conversation
Size Change: +17 B (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
`--wp--preset--${ cssVarInfix }--${ value.slug }: ${ value[ valueKey ] }` | ||
); | ||
} ); | ||
if ( preset ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's interesting. It looks like get
doesn't add enough safety because preset
is defined but set to some falsy value.
Can we check why the preset doesn't have an array value? I don't understand why that happens: if the theme doesn't provide a preset the algorithm uses the core's defaults https://github.com/WordPress/gutenberg/blob/trunk/lib/experimental-default-theme.json#L4 I wonder if there's a bigger issue, something we landed that have broken this. |
@nosolosw it started happening today for themes that are missing |
My point is that to avoid having those checks all over the codebase in every place that data is accessed, we centralize the sanitization logic in the constructor of WP_Theme_JSON, so the other methods know the data is safe to process and don't need to do this check again. |
#30171 might fix it but we need better test coverage for |
I'll go ahead and close this one since #30171 fixes the issue correctly 👍 |
Description
In an FSE theme if some things are missing from theme.json, the site-editor crashes with an error
TypeError: can't access property "forEach", preset is null
. After fixing that there's a 2nd error forpresets
. This PR fixes these 2 so the site-editor can continue loading if the theme doesn't have everything under the sun.To replicate the issue remove the gradients from TT1-blocks' theme.json.
How has this been tested?
Tested with an FSE theme that is missing gradients definitions in theme.json
Types of changes
Added a check to only run the loops if
preset
&presets
are not null.Checklist: