[wp-trac] [WordPress Trac] #59168: Block API: Unnecessary JSON decoding in block parser

[WordPress Trac]

#59168: Block API: Unnecessary JSON decoding in block parser
--------------------------+--------------------------
 Reporter:  dlh           |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  6.4
Component:  Editor        |     Version:  5.0
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:  performance
--------------------------+--------------------------

Comment (by dmsnell):

 Interesting update @dlh.

 By the way @spacedmonkey I was in the process of reviewing this when it
 was merged, so please in the future if you are willing, give me some
 notice to review changes where I'm listed as the code owner, or ask if I
 plan on reviewing them. This one took some extra consideration to verify.

 https://3v4l.org/T4fKB

 The heart of the issue is on `json_encode()` and not on `json_decode()`,
 but since `false` for `$associative` is what makes the difference, I'm
 wondering if historically it was a mistake here to use `true`, thus giving
 the impression that this was superfluous.

 That is, `json_decode( '{}', false )` is not equivalent to `array()`. That
 was true at the time [https://github.com/WordPress/gutenberg/pull/11434/
 GB#11434] merged and it's true today. I think that the augmentation in the
 unit tests is covering up the change, but since everything didn't crash,
 it's probably safe as is.

 That being said, while it seems safe to apply this change now, it's
 incomplete and it would be great if you would be willing to finish by
 updating the spec parser and refactoring the block parser to avoid using
 `$empty_attrs` entirely. Now that there's no more trace of the distinction
 in the code, it's almost silly to continue using this quirk.

 There's certainly no measurable impact on removing the `json_decode()`.

 So I'd be interested in seeing a follow-up that removes the use of
 `$empty_attrs` and associated documentation, and instead creates in-place
 `array()`. If we choose to stick with `$empty_attrs` we still need to
 update the associated documentation.

 This is a fine patch, but it needed a few more steps before merging. Let's
 finish the job 👍

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/59168#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform