Font Face: remove static instance in wp_print_font_faces() #54228
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hellofromtonya
requested review from
ironprogrammer,
anton-vlasenko and
costdev
and removed request for
spacedmonkey
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.4/fonts/fonts.php |
hellofromtonya
added
[Type] Code Quality
This was referenced Sep 6, 2023
|
Flaky tests detected in 5a80d93. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6100934090
|
costdev
approved these changes
Sep 7, 2023
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Sep 7, 2023
The static instance of `WP_Font_Face` is not needed. It was an unnecessary carryover from the experimental Fonts API (which was not introduced into Core). Whereas the Fonts API needed to persist its data (i.e. to maintain the registered and enqueued fonts throughout the web request), Font Face does not have data to persist. Font Face processes the fonts it receives when `WP_Font_Face::generate_and_print( $fonts )` is invoked. Thus, a singleton is not needed. Removing the static reduces the amount of the code in the function and eliminates running its tests in separate processes to ensure a different instance is always used. References: * [WordPress/gutenberg#54228 Gutenberg PR 54228]. Follow-up to [56500]. Props hellofromTonya, costdev. Fixes #59165. git-svn-id: https://develop.svn.wordpress.org/trunk@56540 602fd350-edb4-49c9-b593-d223f7449a82
github-actions bot
pushed a commit
to platformsh/wordpress-performance
that referenced
this pull request
Sep 7, 2023
The static instance of `WP_Font_Face` is not needed. It was an unnecessary carryover from the experimental Fonts API (which was not introduced into Core). Whereas the Fonts API needed to persist its data (i.e. to maintain the registered and enqueued fonts throughout the web request), Font Face does not have data to persist. Font Face processes the fonts it receives when `WP_Font_Face::generate_and_print( $fonts )` is invoked. Thus, a singleton is not needed. Removing the static reduces the amount of the code in the function and eliminates running its tests in separate processes to ensure a different instance is always used. References: * [WordPress/gutenberg#54228 Gutenberg PR 54228]. Follow-up to [56500]. Props hellofromTonya, costdev. Fixes #59165. Built from https://develop.svn.wordpress.org/trunk@56540 git-svn-id: https://core.svn.wordpress.org/trunk@56052 1a063a9b-81f0-0310-95a4-ce76da25c4cd
markjaquith
pushed a commit
to WordPress/WordPress
that referenced
this pull request
Sep 7, 2023
The static instance of `WP_Font_Face` is not needed. It was an unnecessary carryover from the experimental Fonts API (which was not introduced into Core). Whereas the Fonts API needed to persist its data (i.e. to maintain the registered and enqueued fonts throughout the web request), Font Face does not have data to persist. Font Face processes the fonts it receives when `WP_Font_Face::generate_and_print( $fonts )` is invoked. Thus, a singleton is not needed. Removing the static reduces the amount of the code in the function and eliminates running its tests in separate processes to ensure a different instance is always used. References: * [WordPress/gutenberg#54228 Gutenberg PR 54228]. Follow-up to [56500]. Props hellofromTonya, costdev. Fixes #59165. Built from https://develop.svn.wordpress.org/trunk@56540 git-svn-id: http://core.svn.wordpress.org/trunk@56052 1a063a9b-81f0-0310-95a4-ce76da25c4cd
anton-vlasenko
approved these changes
Sep 7, 2023
hellofromtonya
deleted the
fix/remove-static-from-wp_print_font_faces
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See WordPress Core WordPress/wordpress-develop#5159 and commit https://core.trac.wordpress.org/changeset/56540.
What?
Removes the static instance of
WP_Font_Facefromwp_print_font_faces().Why?
This static is an unnecessary carryover from the Fonts API.
Whereas the Fonts API needed to persist its data (i.e. to maintain the registered and enqueued fonts throughout the web request), Font Face does not have data to persist.
Font Face processes the fonts it receives when
::generate_and_print( $fonts )is invoked. Thus, a singleton is not needed.Removing the static reduces the amount of the code in the function and eliminates running its tests in separate processes to ensure a different instance is always used.
How?
Removes the static variable.
Instantiates a new instance each time the function is invoked.
Testing Instructions
There's no functionality changes.
All fonts tests and CI jobs should pass: