[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):

 Thanks @spacedmonkey - by the way, if I'm unresponsive on an issue I
 totally understand moving forward, so thanks for helping out on this.

 > the performance problem

 What performance problem are we talking about?

 > I figured the ship had sailed.

 @dlh same thought. I think there's a chance this only ever exposed itself
 in the tests themselves, as everything else was happy with the loose
 types.

 > I'm happy to try to update the spec parser, but I'm not familiar with
 it. Can you provide a little more guidance about what needs to be updated?

 The spec parser (in the `block-serialization-spec-parser/grammar.pegjs`
 file) leans on `peg_empty_attrs()` to provide this same value. We can
 probably just remove that function and replace all calls to it with
 `array()`.

 > Removing the use of $empty_attrs seems a little more questionable to me.
 It's a public property on a non-final class, and the block_parser_class
 filter makes it possible to use an extended version of that class that
 depends on the property.

 It's probably fine. I doubt anything is relying on it: no public plugins
 are [https://wpdirectory.net/search/01HAD8VT8HMVTBF2W4V2KTSRXD plugin
 search for "empty_attrs"]

 > Arguably, both removing the use of the property within the class and
 removing the property itself are backwards-incompatible changes.

 This is true, and again, probably fine. We can deprecate it if we really
 want to. That would involve changing the internal code while leaving the
 attribute in place, adding a deprecation notice in the docblock comment.

 > the property still provides the code a bit of a semantic boost.

 🤷‍♂️ this seems arguable to me at best. it was added originally to
 address a quirk, and `array()` is common enough that it would be
 surprising if it confused people that it's supposed to represent anything
 other than an empty array.

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