commonL10n and other JS globals removed without backwards compatibility

[kbjohnson90]

In WordPress 5.5, the localized translations under commonL10n were replaced by wp.i18n.__ without deprecation notice or backwards compatibility. Plugins using commonL10n now have JavaScript errors introduced by updating WordPress which can break site functionality.

[kbjohnson90]

commonL10n is not defined

[kbjohnson90]

commonL10n is not defined

[johnbillion]

This was documented in ​https://make.wordpress.org/core/2020/08/05/more-support-for-javascript-i18n-in-wordpress-5-5/ but it's definitely not as explicit as it could be.

I wonder if there's a way we can lazily proxy these variables to their __() equivalent in the future.

[TimothyBlynJacobs]

Replying to johnbillion:

I wonder if there's a way we can lazily proxy these variables to their __() equivalent in the future.

I think we might be able to use ​Object.defineProperty with a get() function that would proxy.

[kbjohnson90]

A backwards compatibility layer for removed localized variables could be added to map the configuration to the translated values in JavaScript, as opposed to PHP.

In this case, the re-usable configuration was replaced with a single use, inline translation.

Instead, the configuration value could be updated to use __() while maintaining re-usability.

var commonL10n = {
    dismiss: __( 'Dismiss this notice.' ),
    ...
}

[peterwilsoncc]

Could something along these lines be added to each affected file?

// First line
window.commonL10n = window.commonL10n || {};

// As needed.
commonL10n.someString = __( 'Some string' );
commonL10n.anotherString = __( 'Another string' );

A back-compat break would remain for any plugins that have filtered the strings upon registration in the script loader, but I suspect that is an edge case.

I am also very slightly concerned such an approach makes the changes in the earlier commits a little redundant as the commonL10n object remains in memory, it simply results in requiring the wp-i18n script as a dependency for each of the files.

[TimothyBlynJacobs]

Replying to peterwilsoncc:

I am also very slightly concerned such an approach makes the changes in the earlier commits a little redundant as the commonL10n object remains in memory, it simply results in requiring the wp-i18n script as a dependency for each of the files.

Does wp-i18n cache translations? If not, there might be performance benefit to storing the translated version in an object.

There is also the question of strings that were changed to use _n since it requires the number of results to give a result.

[joostdevalk]

At the very least we need to make sure in 5.5.1 that this doesn’t cause JS errors, as that breaks other plugins / themes / core that need JS.

[omarreiss]

Added a patch which cleanly deprecates the l10n object getters, throws a console error and returns an empty string whenever a deprecated getter is called.

[SergeyBiryukov]

Related: [48267] / #50526.

[ocean90]

How many plugins does this really affect? Why/how are they using the variable which is intended for core usage only similar to any other translatable string in core?

[joostdevalk]

@ocean90 we've removed a JavaScript global without even proper deprecation. This is a backwards incompatible change, that causes javascript errors, which means that plugins that are not maintained don't just break themselves but can break everything.

If this were PHP, everyone would be crying foul.

[TimothyBlynJacobs]

Why __defineGetter__ instead of defineProperty? It seems like the former isn't encouraged, at least ​according to mdn.

[peterwilsoncc]

@TimothyBlynJacobs defineProperty is unavailable in IE11, whereas __defineGetter__ is.

Given the latter is deprecated, it might be preferable to use some conditional code in a wrapper to prevent errors if any browsers hard deprecate the function.

Reviewing other deprecated JavaScript code, the functionality is generally labelled as such in the docblocks but there isn't an equivalent of PHP's _doing_it_wrong() for JavaScript.

In my view, adding such a function is best left for another ticket.

[desrosj]

Related: #50999.

[TimothyBlynJacobs]

defineProperty is unavailable in IE11, whereas defineGetter is.

@peterwilsoncc is it a particular part of defineProperty that isn't available? According to MDN it's available, ​https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty.

Reviewing other deprecated JavaScript code, the functionality is generally labelled as such in the docblocks but there isn't an equivalent of PHP's _doing_it_wrong() for JavaScript.

It isn't a _doing_it_wrong, but we could potentially use the deprecated package: ​https://github.com/WordPress/gutenberg/tree/7fef85b97c13ab59b47d6d6299e2ba94e9fb00b5/packages/deprecated

[peterwilsoncc]

@TimothyBlynJacobs Must have confused my MDN reading, sorry boss. In that case, I suggest:

• remove console logs for consideration for deprecated JS in another ticket
• switch to defineProperty per Timothy's suggestion above.

[ocean90]

Moving milestone so there's time to identify the possible affected plugins and to discuss other options like updating the plugin/theme.
Returning an empty string might not be the best solution depending on the yet unknown usage of the internal commonL10n strings. Worth to mention that this wasn't the only variable removed during the switch to wp.i18n.

[ocean90]

#51171 was marked as a duplicate.

[ocean90]

Replying to ocean90:

Worth to mention that this wasn't the only variable removed during the switch to wp.i18n.

In #51171 some research was done for this.

[johnbillion]

From #51171:

• wp.updates.l10n
• wpPointerL10n
• commonl10n
• userProfileL10n
• privacyToolsL10n
• authcheckL10n
• wp.themePluginEditor.l10n
• tagsl10n
• adminCommentsL10n
• tagsSuggestL10n
• wpColorPickerL10n
• attachMediaBoxL10n
• postL10n
• inlineEditL10n
• plugininstallL10n
• navMenuL10n
• commentL10n
• setPostThumbnailL10n

[johnbillion]

How about wholesale reverting the JS i18n changes in 5.5 and revisit in 5.6?

[joostdevalk]

I think deprecating these properly is a better idea. A lot of the changes were absolutely for the better, we should have just been a bit more careful with those globals.

[ocean90]

Replying to johnbillion:

How about wholesale reverting the JS i18n changes in 5.5 and revisit in 5.6?

Reverting is definitely not an option as it will cause more trouble (no clean revert possible, translators have to re-translate/approve due do strings moving around, reintroducing i18n issues) than providing a simple fallback/deprecation for the few plugins using the globals as suggested by Joost.

[SergeyBiryukov]

Should we prioritize the deprecation notices for 5.5.1 while we still can?

I agree these i18n changes are great, it just seems like the dev note could have a bit more details, e.g. the list of the removed globals and properties. This also coincided with jQuery Migrate being disabled in the same release, which causes quite a few issues, so adding some fallback seems like a good idea.

[joostdevalk]

@omarreiss expects to have a patch within the hour.

[omarreiss]

Patch deprecates all properties on all removed globals. I use defineProperty now instead of __defineGetter__. If the global has been redefined by a plugin or theme, the properties will still throw a deprecation error, but will still be returned as redefined.

[SergeyBiryukov]

The approach looks good to me and works as expected in my testing.

Made some minor edits to fix JSHint errors, correct some docs, and make the error message translatable.

[peterwilsoncc]

I'm a little concerned about returning empty strings in place of the original text, especially for important notices such as commonL10n.warnDelete and similar that warn an item is about to be permanently deleted. (cc @SergeyBiryukov following our conversation in Slack regarding this)

In 51123.2.diff​

• Deprecated objects include the original text -- where the translation was changed in the original commit, I've used the new string to avoid causing problems for the translation teams
• Switched console.error to console.warn as these match deprecation warnings used in the version of React shipped with WordPress, refer ​https://github.com/facebook/react/issues/9395
• window.tagsl10n appears to have been named window.tagsSuggestL10n in 5.5 so I have renamed that to match in the updated patch
• wpWidgets.l10n wasn't included in the previous patches, I have added it to src/js/_enqueues/admin/widgets.js

I'm after logic checks on the following:

• the use of pagenow for network and single site plugins
• how I have used the translation functions
• placement of translations will not cause extra work per @ocean90's concerns above

[ocean90]

Replying to peterwilsoncc:

Thanks for your work on the patch but I think it's not responsible to ship such a performance penalty in core for everyone.
We're still looking at plugins which are using "private" globals which unfortunately were required to pass core translations to scripts. Just because they are there it doesn't mean that they are supported in any way. Otherwise there would have been some proper documentation. #50976 is a similar recent example for breaking something that wasn't intended to work.
I also took a look at some of the plugins and some don't even define the necessary handle as a script dependencies neither do they the check for their existence before accessing the globals. They just expect them to be there…
With regards to your example commonL10n.warnDelete, that's definitely something we shouldn't have to care about because the proper API would be using window.showNotice.warn().

Though, I do understand the issue that due to a few plugins other plugins or even core may not working properly anymore. And that's something we should fix by providing a graceful fallback as suggested in 51123.diff​.
I'm only wondering why the globals have all been defined in common.js? This will make the globals suddenly available on all screens. Wouldn't it be better to add them to each file where they were used previously?

Unlike in PHP, the now deprecated and possible unused code would get shipped to the client on every (uncached) page load. So, we should include this with a clear deprecation policy. For example the current fallback will only be included in the next two major versions but you can install plugin X (similar to ​Enable jQuery Migrate Helper) to keep the fallback for a bit longer. We could use ​@wordpress/deprecated for the extended message.

[peterwilsoncc]

Yeah, the performance consideration is fair enough and something I started to wonder as I moved toward finishing the patch but I figured I'd share it.

51123.2.diff​ can be discarded as an interesting experiment.

As 51123.diff​ is built upon the following will need to be done:

• window.tagsl10n renamed window.tagsSuggestL10n
• wpWidgets.l10n will need to be added
• further work as mentioned above.

Thanks for reviewing @ocean90

[joostdevalk]

Unlike in PHP, the now deprecated and possible unused code would get shipped to the client on every (uncached) page load. So, we should include this with a clear deprecation policy.

I fully support this. Two versions seems good to me too, should we do a make post to discuss that? Or can that just be decided in this thread?

[omarreiss]

Reasons why I think 51123.diff should be committed for 5.5.1 (with the missed vars @peterwilsoncc noted).

• It's a no risk change and solves the issues. Even takes into account potential plugins who redefine the removed globals.
• Returning empty strings is fine. The worst thing that can happen is that these strings (which shouldn't have been used by integrators in the first place) render empty. Much better than breaking execution and actually nudges integrators to update.
• A deprecation policy for JavaScript is definitely something that should be worked on. Gutenberg already has one that seems to work fine: ​https://developer.wordpress.org/block-editor/developers/backward-compatibility/deprecations/. There's no reason this should block commit though.
• The reason I defined most globals in common.js is to keep the fix as simple as possible. common.js gets loaded mostly anywhere in the admin and since these globals are deprecated, we don't have to worry about them being overly present. The impact of that is negligible and it makes cleanup later a more simple task. In places where only an l10n property was removed instead of the entire global, I had to do it in the file where that actually happens, since the global is still localized onto that script.

[SergeyBiryukov]

51123.3.diff​ addresses a few points from comment:32 and comment:34:

• Switch console.error() to console.warn().
• Rename duplicate window.tagsl10n object to window.tagsSuggestL10n.
• Add missing wpWidgets.l10n object.

I think this is ready for commit now.

[desrosj]

A few of the variables are missing @since tags.

• setPostThumbnailL10n
• navMenuL10n
• plugininstallL10n
• postL10n
• userProfileL10n

I don't think this is a blocker for commit though. RC2 should be going out later today, so having this merged for testing is preferable. The @since values for each of those can be researched and the docs can be updated later.

[SergeyBiryukov]

Replying to desrosj:

I don't think this is a blocker for commit though. RC2 should be going out later today, so having this merged for testing is preferable. The @since values for each of those can be researched and the docs can be updated later.

Thanks for the review! 51123.4.diff​ adds the missing @since tags.

[SergeyBiryukov]

In 48923:

Script Loader: Add backward compatibility for JavaScript i18n globals and properties deprecated in WordPress 5.5.

The recommended approach for any plugins using these globals or properties is to switch to the newer wp.i18n functions.

In the meantime, this ensures that accessing any of these globals does not break the rest of the code on the page, and an appropriate warning message is logged to the JavaScript console.

Follow-up to: https://core.trac.wordpress.org/query?summary=~wp.i18n&milestone=5.5

Props omarreiss, peterwilsoncc, kbjohnson90, johnbillion, TimothyBlynJacobs, joostdevalk, ocean90, desrosj, SergeyBiryukov.
Fixes #51123.

[SergeyBiryukov]

Reopening for backporting to the 5.5 branch after a second committer's review.

[SergeyBiryukov]

Follow-up: #51200

[desrosj]

Looks good to backport.

[desrosj]

In 48925:

Script Loader: Add backward compatibility for JavaScript i18n globals and properties deprecated in WordPress 5.5.

The recommended approach for any plugins using these globals or properties is to switch to the newer wp.i18n functions.

In the meantime, this ensures that accessing any of these globals does not break the rest of the code on the page, and an appropriate warning message is logged to the JavaScript console.

Follow-up to: https://core.trac.wordpress.org/query?summary=~wp.i18n&milestone=5.5

Props omarreiss, peterwilsoncc, kbjohnson90, johnbillion, TimothyBlynJacobs, joostdevalk, ocean90, desrosj, SergeyBiryukov.
Merges [48923] to the 5.5 branch.
Fixes #51123.

[SergeyBiryukov]

Follow-up: #51317