Make WordPress Core

Opened 2 months ago

Closed 2 months ago

Last modified 2 weeks ago

#60761 closed defect (bug) (fixed)

Interactivity API: Empty state objects are not correctly sent to the client

Reported by: jonsurrell's profile jonsurrell Owned by: swissspidy's profile swissspidy
Milestone: 6.5 Priority: normal
Severity: normal Version: 6.5
Component: Interactivity API Keywords: has-patch has-unit-tests commit dev-reviewed fixed-major
Focuses: Cc:

Description

If an Interactivity API store is created on the server with an empty array:

wp_interactivity_state( 'myPlugin', array() );

It is serialized and sent to the client as a JavaScript array: []. However, state is expected to be an object {}. This creates problems on the client when server and client states are merged. For example, it breaks state getters defined on the client like this:

store( "myplugin/demo", {
  state: {
    get aValue() {
      return 'This will break.';
    },
  }
} );

Change History (15)

This ticket was mentioned in PR #6261 on WordPress/wordpress-develop by @jonsurrell.


2 months ago
#1

  • Keywords has-patch has-unit-tests added

Prevent an empty Interactivity store from being serialized as []. Stores are expected to be JSON objects ({}), but PHP will json_encode empty arrays as []. An option would be to force PHP to use empty objects with JSON_FORCE_OBJECT, but that is deep, so if we wanted e.g. array( 'myStore' => array( 'ids' => array() ) ) the inner array() would also become {}, which we probably don't want.

Trac ticket: https://core.trac.wordpress.org/ticket/60761

@jonsurrell commented on PR #6261:


2 months ago
#2

👋 @c4rl0sbr4v0 @DAreRodz for review.

#3 @swissspidy
2 months ago

  • Component changed from General to Editor
  • Milestone changed from Awaiting Review to 6.5
  • Version set to trunk

@jonsurrell commented on PR #6261:


2 months ago
#4

This pairs nicely with https://github.com/WordPress/gutenberg/pull/59842 which will ensure stores are initialized on the client before they're needed.

#5 follow-up: @gziolo
2 months ago

What's the implication of serving an empty array [] from the server instead of an object {}? Will it break something on the client if the client-side patch lands? I'm thinking about how severe the problem is to make a good decision whether to include in 6.5.0.

Last edited 2 months ago by gziolo (previous) (diff)

#6 in reply to: ↑ 5 @jonsurrell
2 months ago

Replying to gziolo:

What's the implication of serving an empty array [] from the server instead of an object {}? Will it break something on the client if the client-side patch lands? I'm thinking about how severe the problem is to make a good decision whether to include in 6.5.0.

In my opinion this fix is important and should be landed for 6.5. I'd like https://github.com/WordPress/gutenberg/pull/59842 to also be backported to 6.5. I think those two changes together are sufficient to address the related problems I observed.

The client expects state to be a JavaScript object. I explained some of this on the PR in this comment, but I'll summarize here.

The client expects state to be a JavaScript object ({}) and does not currently do any validation on the initial state provided by the server, or any state the first time a store is created for a given namespace. Checks should be added to the client, but that may be a bigger change. Both the client and the server should be fixed to prevent non-objects from being stored in state.

I've observed this problem of an array entering state breaking plugin code on the client. Looking at the code, it seems like the problem is the following:

I'm happy to also fix non-objects being stored in state on the first store call, that would be another client side PR. If that is fixed on the client, this change would become more of an optimization than a bugfix. It would still be incorrect to send [] state to the client, but it wouldn't be harmful.

#7 @jonsurrell
2 months ago

Here's a PR for the client-side fix to prevent non-objects from being stored as interactivity store state: https://github.com/WordPress/gutenberg/pull/59886

#8 @gziolo
2 months ago

  • Keywords dev-feedback added

@swissspidy, it would be important to land this bug fix in 6.5.0 to avoid unexpected issues with custom blocks that set the initial state to values that are incorrectly serialized.

#9 @swissspidy
2 months ago

  • Keywords commit added
  • Owner changed from jonsurrell to swissspidy
  • Status changed from assigned to reviewing

#10 @swissspidy
2 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 57841:

Interactivity API: Do not print state if it’s an empty array.

This prunes stores and configurations that are empty arrays, as stores are expected to be JSON objects.
By not printing empty configurations, less redundant data is serialized into the HTML.

Props jonsurrell, luisherranz, darerodz, gziolo, swissspidy.
Fixes #60761.

#12 @swissspidy
2 months ago

  • Keywords dev-reviewed added; dev-feedback removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

#13 @swissspidy
2 months ago

  • Keywords fixed-major added

#14 @swissspidy
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57843:

Interactivity API: Do not print state if it’s an empty array.

This prunes stores and configurations that are empty arrays, as stores are expected to be JSON objects.
By not printing empty configurations, less redundant data is serialized into the HTML.

Reviewed by gziolo.
Merges [57841] to the to the 6.5 branch.

Props jonsurrell, luisherranz, darerodz, gziolo, swissspidy.
Fixes #60761.

#15 @jonsurrell
2 weeks ago

  • Component changed from Editor to Interactivity API
Note: See TracTickets for help on using tickets.