WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.4k stars 4.16k forks source link

Undefined (null) site_logo breaks plugins depending on wp.api.models.Settings validation #37808

Open dingo-d opened 2 years ago

dingo-d commented 2 years ago

Description

I was testing my plugin with the 5.9-RC1-52542 version of WP, and the new twentytwentytwo theme, and noticed a weird bug.

I'm building my options page with React, so that I can reuse GB components. For that purpose I'm using wp.api.models.Settings() to fetch my plugin's settings, and to save them (this.settings.save(options, {...})).

And on plugins save, I got an error in one of my helper methods

    hasErrorClass(type) {
        return this.state.errors.hasOwnProperty(type) ? 'has-error' : '';
    }

TypeError: Cannot read properties of undefined (reading 'hasOwnProperty')

I thought that was odd since I've never seen this error before. Then I checked at the network tab and noticed that my settings call threw an error

POST ttps://wp-testing.test/wp-json/wp/v2/settings/

{"code":"rest_invalid_stored_value","message":"The site_logo property has an invalid stored value, and cannot be updated to null.","data":{"status":500}}

This definitely wasn't coming from my plugin (all my options are prefixed). So I thought to myself: hmmm, site_logo is a theme thing, usually set in customizer or something like that. So I went to the new Editor screen for the FSE, and set the header, and used it as a site logo, and lo and behold no errors in my plugin 😄 .

So I think that these FSE options that are stored in the options table should have some kind of default values, to prevent these kinds of issues down the line.

Because to the regular user, this would look like an issue in my plugin, and it's not (since I'm also setting defaults for my plugin). It's an issue with the theme/FSE features.

Step-by-step reproduction instructions

  1. Set up Wp 5.9 with FSE
  2. Activate twentytwentytwo theme
  3. Install my plugin (it depends on woocommerce so you'll need that as well)
  4. Try to update any plugin settings prior to setting anything in the theme

Screenshots, screen recording, code snippet

Network tab and errors

image image

Prior to saving:

image

After saving (WSOD)

image

Environment info

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 commented 2 years ago

Hi, @dingo-d

I'm not very familiar with wp.api.models.Settings() but it's interesting that this.settings.save sends default WP options as payload.

Can you provide a sample JS code to replicate the issue?

dingo-d commented 2 years ago

Ok, so here's an MVP plugin that reproduces the same for me. This is the test.php file

<?php

/**
 * Plugin main file starting point
 *
 * Plugin Name:          Test
 * Description:          Test
 * Version:              1.0.0
 */

add_action('admin_enqueue_scripts', function () {
    wp_register_script(
        'test-js',
        plugins_url() . '/test/test.js',
        [
            'wp-api',
            'wp-i18n',
            'wp-components',
            'wp-element',
            'wp-api-fetch',
        ],
        '1.0.0',
        true
    );

    wp_enqueue_script('test-js');
});

add_action('admin_menu', function () {
    add_menu_page(
        'Test',
        'Test',
        'edit_posts',
        'test',
        function () {
            ?>
            <div class="wrap">
                <div id="test-page"></div>
            </div>
            <?php
        }
    );
});

And this is the test.jsx file

```jsx /** * WordPress dependencies */ const {__} = wp.i18n; const {apiFetch} = wp; const { Button, Spinner, } = wp.components; const { render, Fragment, Component, createInterpolateElement } = wp.element; const defaultState = { isLoading: true, isApiRequestOver: true, isSaving: false, isSaved: false, hasErrors: false, errors: {} }; class App extends Component { constructor() { super(...arguments); this.state = defaultState; this.settings = {}; this.setStateSynchronous = this.setStateSynchronous.bind(this); this.updateOptions = this.updateOptions.bind(this); } componentDidMount() { wp.api.loadPromise.done(() => { if (this.state.isLoading) { this.settings = new wp.api.models.Settings(); this.settings.fetch().then(res => { this.setState({ isLoading: false, }); }); } }); } setStateSynchronous(stateUpdate) { return new Promise(resolve => { this.setState(stateUpdate, () => resolve()); }) } async updateOptions() { await this.setStateSynchronous( state => ({isSaving: true, errors: {}}) ); // Create options object. const options = {}; this.settings.save(options, { success: (model, res) => { Object.keys(options).map((option) => { this.setState({ option: res[option], isSaving: false, isSaved: true, hasErrors: false, }); }); setTimeout(() => { this.setState({ isSaved: false }); }, 3000); }, error: (model, res) => { const errors = res.responseJSON.data.params; this.setState({ errors: errors, isSaving: false, isSaved: true, hasErrors: true, }); const target = document.querySelector(`[name=${Object.keys(errors)[0]}]`); window.scrollTo({ top: target.offsetTop - 30, left: 0, behavior: 'smooth', }); setTimeout(() => { this.setState({ isSaved: false }); }, 3000); } }); } hasErrorClass(type) { return this.state.errors.hasOwnProperty(type) ? 'has-error' : ''; } render() { if (this.state.isLoading) { return (
); } return (
{this.state.isSaving ? : ''}
); } } render( , document.getElementById('test-page') ); ```

I'm using babel-cli to compile it to test.js

I have the following package.json

{
  "dependencies": {
    "babel-cli": "^6.26.0",
    "babel-preset-react": "^6.24.1"
  }
}

And I'm using npx babel --presets react test.jsx -o test.js command.

When I click save button the following request payload is being send to the settings endpoint

{
    "title": "WP Testing",
    "description": "Just another WordPress site",
    "url": "https://wp-testing.test",
    "email": "admin@test.test",
    "timezone": "",
    "date_format": "F j, Y",
    "time_format": "g:i a",
    "start_of_week": 1,
    "language": "en_US",
    "use_smilies": true,
    "default_category": 1,
    "default_post_format": "0",
    "posts_per_page": 10,
    "default_ping_status": "open",
    "default_comment_status": "open",
    "site_logo": null, // HERE'S THE ISSUE
    "site_icon": 55
}

The settings are part of the WP core models: https://developer.wordpress.org/rest-api/using-the-rest-api/backbone-javascript-client/

Hope this helps.