Skip to content

Use static closures when not using $this#50723

Merged
westonruter merged 2 commits intotrunkfrom
add/static-closures
May 31, 2023
Merged

Use static closures when not using $this#50723
westonruter merged 2 commits intotrunkfrom
add/static-closures

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented May 17, 2023

What?

Add static keyword to closures that don't use $this

Why?

Since closures capture a reference to $this, memory leaks can ensue.

This applies the same change that was just made in Core-58323

How?

I used the SlevomatCodingStandard.Functions.StaticClosure sniff to apply the change with phpcbf.

Testing Instructions

n/a

Testing Instructions for Keyboard

n/a

Screenshots or screencast

@westonruter westonruter added the [Type] Performance Related to performance efforts label May 17, 2023
@gziolo
Copy link
Copy Markdown
Member

gziolo commented May 19, 2023

I see that @aristath opened another PR #50657 with similar changes.

CI errors reported are unrelated to this PR. I belive it must be reported on trunk, too.

@aristath aristath mentioned this pull request May 19, 2023
Closed
@aristath
Copy link
Copy Markdown
Member

aristath commented May 19, 2023

I see that @aristath opened another PR #50657 with similar changes.

Yes, it was almost identical, inspired by @westonruter's PR in WordPress/performance#729 👍
I closed the other PR, we can continue with this one 🎉

spacedmonkey
spacedmonkey reviewed May 19, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

This is nearly there. I have one nitpic, then I will approve.

@gziolo
Copy link
Copy Markdown
Member

gziolo commented May 19, 2023

This is nearly there. I have one nitpic, then I will approve.

Once approved, it can be merged with the admin permissions if the JS linting errors are the only blocker in the CI check.

@westonruter westonruter requested a review from spacedmonkey May 19, 2023 16:34
@westonruter
Copy link
Copy Markdown
Member Author

westonruter commented May 19, 2023

@aristath Sorry that I didn't check for your PR first!

What do we think about including slevomat/coding-standard in the phpcs.xml.dist? It was discouraged from core in WordPress/wordpress-develop#4457 for Core-58323.

@westonruter
Merge branch 'trunk' of https://github.com/WordPress/gutenberg into a…
89b582f 
…dd/static-closures

* 'trunk' of https://github.com/WordPress/gutenberg: (26 commits)
  Add transparent outline to input control BackdropUI focus style. (#50772)
  Added wrapper element for RichText in File block (#50607)
  Remove the experimental flag of the command center (#50781)
  Update the document title in the site editor to open the command center (#50369)
  Remove `unwrap` from transforms and add `ungroup` to more blocks (#50385)
  Add new experimental version of DropdownMenu (#49473)
  Force display of in custom css input boxes to LTR (#50768)
  Polish experimental navigation block (#50670)
  Support negation operator in selectors in the Interactivity API (#50732)
  Minor updates to theme.json schema pages (#50742)
  $revisions_controller is not used. Let's delete it. (#50763)
  Remove OffCanvasEditor (#50705)
  Mobile - E2E test - Update code to use the new navigateUp helper (#50736)
  Try: Smaller external link icon (#50728)
  Block Editor: Remove unused 'useIsDimensionsSupportValid' method (#50735)
  Fix flaky media inserter drag-and-dropping e2e test (#50740)
  docs: Fix change log typo (#50737)
  Edit Site: Fix `useEditedEntityRecord()` loading state (#50730)
  Fix labelling, description, and focus style of the block transform to pattern previews (#50577)
  Fix Global Styles sidebar block selection on zoom out mode (#50708)
  ...
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 19, 2023

Flaky tests detected in 89b582f.
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/5026422903
📝 Reported issues:

@westonruter
Copy link
Copy Markdown
Member Author

westonruter commented May 26, 2023

So include slevomat/coding-standard or not?

aristath
aristath approved these changes May 31, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

This looks good to me 👍

So include slevomat/coding-standard or not?

I suppose we could merge the improvements from this PR without including slevomat/coding-standard just to get this part out of the way, and create a separate issue to include the slevomat/coding-standard package where we can focus more on that discussion 👍

@westonruter westonruter merged commit d665608 into trunk May 31, 2023
@westonruter westonruter deleted the add/static-closures branch May 31, 2023 17:15
@github-actions github-actions bot added this to the Gutenberg 16.0 milestone May 31, 2023
@ramonjd ramonjd mentioned this pull request Jun 5, 2023
Closed
@ramonjd ramonjd added the Needs PHP backport Needs PHP backport to Core label Jun 5, 2023
@westonruter westonruter mentioned this pull request Jun 16, 2023
Closed
pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Jun 26, 2023
@westonruter
Editor: Use static closures to avoid memory leaks.
c7597fa 
Backports Gutenberg changes from WordPress/gutenberg#50723.

Amends [55822].

Fixes #58323.
Props westonruter, spacedmonkey, flixos90. 


git-svn-id: https://develop.svn.wordpress.org/trunk@56038 602fd350-edb4-49c9-b593-d223f7449a82
github-actions bot pushed a commit to gilzow/wordpress-performance that referenced this pull request Jun 26, 2023
@westonruter
Editor: Use static closures to avoid memory leaks.
366bf22 
Backports Gutenberg changes from WordPress/gutenberg#50723.

Amends [55822].

Fixes #58323.
Props westonruter, spacedmonkey, flixos90. 

Built from https://develop.svn.wordpress.org/trunk@56038


git-svn-id: https://core.svn.wordpress.org/trunk@55550 1a063a9b-81f0-0310-95a4-ce76da25c4cd
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Jun 26, 2023
@westonruter
Editor: Use static closures to avoid memory leaks.
33f71c4 
Backports Gutenberg changes from WordPress/gutenberg#50723.

Amends [55822].

Fixes #58323.
Props westonruter, spacedmonkey, flixos90. 

Built from https://develop.svn.wordpress.org/trunk@56038


git-svn-id: http://core.svn.wordpress.org/trunk@55550 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 27, 2023
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
@westonruter @sethrubenstein
Use static closures when not using $this (WordPress#50723)
722aca3
@Mamaduka Mamaduka mentioned this pull request Jul 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spacedmonkey spacedmonkey spacedmonkey approved these changes

@aristath aristath aristath approved these changes

@ntwb ntwb Awaiting requested review from ntwb ntwb is a code owner

@nerrad nerrad Awaiting requested review from nerrad nerrad is a code owner

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@tellthemachines tellthemachines Awaiting requested review from tellthemachines tellthemachines is a code owner

Assignees

No one assigned

Labels

[Type] Performance Related to performance efforts

Projects

None yet

Milestone

Gutenberg 16.0

Development

Successfully merging this pull request may close these issues.

5 participants

@westonruter @gziolo @aristath @spacedmonkey @ramonjd