Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script loader 6.4 compat: check for init hook completion #58406

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Jan 29, 2024

What, why, how?

Core checks that the init hook is complete before trying to use the global $wp_locale value in wp_default_packages_inline_scripts, which is where the date settings are set for inline JS.

This change backports that from Core.

Note

No Core backport required as the changes from #53931 were ported over in WordPress/wordpress-develop#5101

Testing Instructions

Test steps in #53931 should work as expected.

…lobal $wp_locale value in wp_default_packages_inline_scripts, which is where the date settings are set for inline JS. This change backports that from Core.
@ramonjd ramonjd added the Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core label Jan 29, 2024
@ramonjd ramonjd self-assigned this Jan 29, 2024
Copy link

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/script-loader.php

@ramonjd ramonjd added Internationalization (i18n) Issues or PRs related to internationalization efforts [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. labels Jan 29, 2024
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this up @ramonjd!

This looks like a good way to do it to me, as it follows how it's guarded in core 👍

✅ Confirmed that wp.date.getSettings() works as before in the site editor, returning translated settings
✅ Double-checked that $wp_locale in core is set to an instance of WP_Locale, which sets public properties with empty arrays, so if $wp_locale is set, then we don't need to check if the properties are arrays before accessing them
✅ Double-checked that the locale is instantiated set before init here: https://github.com/WordPress/wordpress-develop/blob/a61525049f574a826cf20a8b1b624e1b42970b10/src/wp-settings.php#L613

LGTM! ✨

@ramonjd
Copy link
Member Author

ramonjd commented Jan 29, 2024

Appreciate the lightning fast review @andrewserong 🙇🏻

@ramonjd ramonjd merged commit fa802d1 into trunk Jan 29, 2024
61 checks passed
@ramonjd ramonjd deleted the update/wp_date_settings-script-6.4-runtime branch January 29, 2024 22:40
@github-actions github-actions bot added this to the Gutenberg 17.7 milestone Jan 29, 2024
@MaggieCabrera MaggieCabrera added the [Package] Date /packages/date label Feb 9, 2024
fullofcaffeine pushed a commit that referenced this pull request Feb 12, 2024
…lobal $wp_locale value in wp_default_packages_inline_scripts, which is where the date settings are set for inline JS. This change backports that from Core. (#58406)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core Internationalization (i18n) Issues or PRs related to internationalization efforts [Package] Date /packages/date [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants