Skip to content

Remove empty attrs.#54496

Merged
mikachan merged 2 commits intotrunkfrom
fix/types
Oct 13, 2023
Merged

Remove empty attrs.#54496
mikachan merged 2 commits intotrunkfrom
fix/types

Conversation

@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Sep 15, 2023

What?

Following work in #54424, this replaces the $empty_attrs object, which used to have a role, with an empty array wherever it was previously used.

Why?

The reason for having the property was that the block parser used to create a special object for empty attributes to disambiguate an empty object and an empty array in JSON. Because the block parser no longer creates this special object there's no more need to have an $empty_attrs property. It's not causing harm, but its continued presence might confuse people who are wondering what its purpose is.

How?

Directly replaces the property lookup with in-place array() construction.

Testing Instructions

A basic smoke test should verify failure because any problem in the block parser is likely to break a site in obvious ways. Ensure that the test suite passes.

@github-actions
Copy link

Flaky tests detected in 2cc7fc5.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6197299771
📝 Reported issues:

@dmsnell dmsnell added [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f Needs PHP backport Needs PHP backport to Core labels Sep 15, 2023
Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me, thanks @spacedmonkey for following-up on #54496 #54424.

I'm leaving a pre-emptive approval on this because all that remains is filling in the PR description and taking care of some styling issues that CI will reject.

@mikachan
Copy link
Member

👋 @dmsnell This is on the 6.4 Editor Tasks board, so I'm wondering if this needs to be merged ready for the upcoming 6.4 RC. As @spacedmonkey is away at the moment, I wondered if you could help here. I'm happy to help fix the CS issues, but I'm not sure how high a priority this is or if it's not ready to be merged yet. Thank you!

@dmsnell
Copy link
Member

dmsnell commented Oct 13, 2023

@mikachan there is no urgency here, but it's ready apart from the styling linter not liking it. It's not a behavioral change so it shouldn't cause any hassle with or without it; it's just a follow-up to #54424 without a description, explanation, instructions, or links. I'll add a description.

@mikachan mikachan added Backport to WP Beta/RC and removed Needs PHP backport Needs PHP backport to Core labels Oct 13, 2023
@mikachan mikachan merged commit 8fa0d97 into trunk Oct 13, 2023
@mikachan mikachan deleted the fix/types branch October 13, 2023 23:43
@github-actions github-actions bot added this to the Gutenberg 16.9 milestone Oct 13, 2023
SiobhyB pushed a commit that referenced this pull request Oct 16, 2023
* Remove empty attrs.

* Fix linter errors

---------

Co-authored-by: Sarah Norris <sarah@sekai.co.uk>
@SiobhyB
Copy link
Contributor

SiobhyB commented Oct 16, 2023

I just cherry-picked this PR to the 6.4-rc1-2 branch to get it included in the next release: 0afdb21

SiobhyB pushed a commit that referenced this pull request Oct 16, 2023
* Add selector as id to layout style overrides. (#55291)

* Fix flickering when focusing on global style variations (#55267)

* ProgressBar: use text color to ensure enough contrast against background (#55285)

* Use text color at different opacities for track and indicator

* Add high contrast mode styles

* CHANGELOG
# Conflicts:
#	packages/components/CHANGELOG.md

* Remove empty attrs. (#54496)

* Remove empty attrs.

* Fix linter errors

---------

Co-authored-by: Sarah Norris <sarah@sekai.co.uk>

* Add IS_GUTENBERG_PLUGIN flag to LastRevision (#55253)

* useBlockPreview: Try outputting EditorStyles to ensure local style overrides are rendered (#55288)

* useBlockPreview: Try alternative fix for displaying local style overrides

* Avoid duplicate styles, fix rendering issues in Safari

* Add more explanatory comments

* Remove additional check for styles within the block preview, as it is not needed since EditorStyles handles its own style overrides retrieval

* Bug: PHP notice when an image with lightbox is deleted (#55370)

* Fix PHP notice when an image with lightbox is deleted

* Fix PHP notice when an image with lightbox is deleted

---------

Co-authored-by: tellthemachines <tellthemachines@users.noreply.github.com>
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Co-authored-by: Kishan Jasani <kishanjasani007@yahoo.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f [Type] Performance Related to performance efforts

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants