-
Notifications
You must be signed in to change notification settings - Fork 3
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
Avoid loading translations too early #56
Conversation
Signed-off-by: Moritz Meißelbach <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #56 +/- ##
============================================
+ Coverage 98.80% 98.84% +0.04%
Complexity 251 251
============================================
Files 10 10
Lines 586 607 +21
============================================
+ Hits 579 600 +21
Misses 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
We should also update the documentation and mention that the data in |
Signed-off-by: Moritz Meißelbach <[email protected]>
Signed-off-by: Moritz Meißelbach <[email protected]>
@Chrico Added! |
Sorry I need to jump in with another request...first of all: I think this is a breaking change for existing Packages, in case they are using something from the We should not break the default behavior, but at least fix the In order to stay "backward compatible", we should add a check for the WP version to be There are probably some Plugins out there, which are booted late - so after |
Your request makes sense and according to how I understand the situation, this should be a working approach, since it is essentially the same check that WP does before throwing the |
Add tests and improve docs
This reverts commit 2108ee2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed internally, we decided for not auto translating the Plugin Properties. A check by WordPress version and current action/action done would cause an inconsistent result.
We will keep track of this hot topic in WordPress Core and see how we can improve this in future. For now it is always possible to call in code:
esc_html_e( $pluginProperties->description(), $pluginProperties->textDomain() );
when a translated field is required.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix
What is the current behavior? (You can also link to an open issue here)
WordPress 6.7 removed the need to
load_plugin_textdomain
manually.In return, we receive a notice if translations are loaded/accessed too early.
What is the new behavior (if this is a feature change)?
Passing
$translate = false
in the call toget_plugin_data
avoids the_doing_it_wrong
and the resulting noticeSince
ExecutableModule
is responsible for hooking module logic into WordPress, it makes sense to bootstrap the package itself as early as feasible.Hence, deferring bootstrap to
after_setup_thene
orinit
as proposed by core severely limits our flexibility in terms of what modules can to in the "remaining time"Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No.
Other information: