I18n messages from Composer packages are never translated

anomiex commented 3 years ago

Impacted plugin

Jetpack, Backup, Boost

Steps to Reproduce

Easiest way to observe this at the moment is to compare the list of strings available for translation for the Backup plugin at https://translate.wordpress.org/projects/wp-plugins/jetpack-backup/dev/es/default/ with the strings that are present in packages/backup.

Or you could run wp i18n make-pot on one of the plugins and look more directly.

A clear and concise description of what you expected to happen.

All the strings from the package are able to be translated, somehow.

What actually happened

Only the ones defined in the plugin itself can be translated.

Other information

The problem is that wp i18n make-pot ignores vendor/.

There was some discussion at p1634933470116600-slack-CBG1CP4EN.

One idea would be to configure composer to use something other than vendor/ for the plugins that bundle vendor/. But it was said that this would need some work in the Jetpack Autoloader package.

Another idea would be to somehow get the packages translated as standalone projects, and then either bundle the translations in the packages or somehow convince WordPress to download the packages' i18n data along with the plugin. There doesn't seem to be a straightforward way to do this at the moment.

Operating System

Other

OS Version

No response

Browser

Other / Not applicable

Browser Version(s)

No response

jsnmoon commented 3 years ago

This issue will most likely block @Automattic/jetpack-search from moving forward with our code migration to the standalone package. Does @Automattic/jetpack-crew have an idea on when/if this issue will be addressed?

For context, we're aiming to complete our code migration to the standalone package by the end of this month.

cc @bluefuton

kangzj commented 3 years ago

Probably still a work around. Just a thought here, we could possibly publish the JS builds to _inc/build, i.e. folders that make-pot doesn't ignore and git wouldn't version by using a env variable or sth. For PHP, the same thing.

jeherve commented 3 years ago

Only the ones defined in the plugin itself can be translated.

Is the problem linked to the files in /vendor, or the files using a textdomain (jetpack) that is not the plugin's defined textdomain, as discussed in #20320 and #18070?

One idea would be to configure composer to use something other than vendor/ for the plugins that bundle vendor/.

If /vendor is indeed excluded, would it be an option to suggest that it stop being excluded?

kraftbj commented 3 years ago

vendor is being excluded by Core's make-pot. IMO, pushing Core/WP CLI to not exclude vendor for the plugins directory is the most stable way forward.

@anomiex Where do things stand on needing a dynamic textdomain (e.g. what textdomain is used for a package that is in both Jetpack-the-plugin and a standalone)? Whether or not vendor is excluded, IIRC, we'll run into issues if there is a different textdomain.

kraftbj commented 3 years ago

On the WP CLI side, the default exclusion of vendor was last brought up (that I could see) in https://github.com/wp-cli/i18n-command/issues/172

If WP CLI does not want to entertain a change to the default, we could open a meta.trac ticket regarding .org including vendor (or perhaps having some way for a plugin to indicate to include vendor, or a specific folder string like vendor/automattic/ , etc

kraftbj commented 3 years ago

Lastly, going back to how we did JS translations could work. On build, we make-pot ourselves, convert that to an array in PHP that has the plugin textdomain and include that.

jeherve commented 3 years ago

On build, we make-pot ourselves, convert that to an array in PHP that has the plugin textdomain and include that.

I was happy when we were able to move away from this towards core js translations, but I can see how useful it could be here. Such a system may allow us to address both this issue and #20320 at once, when building the production version of a plugin generated from the monorepo.

kraftbj commented 3 years ago

I was happy when we were able to move away from this towards core js translations, but I can see how useful it could be here.

Yeah, I really don't like having to go back to a homegrown solution, but given that the solutions otherwise are to get Core or WP CLI to move on something when we're looking at wanting something ready to go into prod at the end of month... I think homegrown may be the best way forward until the ecosystem (with our help) can have a solution.

anomiex commented 3 years ago

I'm supposed to be AFK today, but I was thinking about this anyway and had a few ideas.

A composer custom installer could put just the relevant packages outside of vendor/, assuming we're willing to identify them all with an appropriate "type" field in their composer.json files. That might avoid the trouble with the autoloader since all the metadata would still be in vendor/.
It looks like we could get away with not actually mangling the domains in the packages.
- When WordPress.org's infrastructure calls wp i18n make-pot, it passes the --ignore-domain option. So the strings will get picked up regardless of the domain the package uses, all will be placed in the plugin's pot file (so under the plugin's domain).
- The trick then would be to somehow map all the packages' domains to the plugin's domain, so when the package does __( 'some test', 'package-domain' ) that it loads from the plugin's domain instead. I don't see any direct way to alias domains in WordPress, there's no filter in the get_translations_for_domain() code path, but we could use the gettext, ngettext, gettext_with_context, and ngettext_with_context filters to make it happen for each message. The gettext filter function might look something like
```
public function gettext_filter_alias( $translation, $text, $domain ) {
if ( $translation === $text && isset( $this->aliases[$domain] ) ) {
    return translate( $text, $this->aliases[$domain] );
}
return $translation;
}
```
  The others would be similar. For the JS, we could use load_script_translation_file to swap the domains in the filename.
- If all that works, then all we need is to build the alias map. We could have the autoloader collect the list of packages' domains from the packages' composer.jsons, then the plugin would use that to register the alises.

Thoughts?

Longer term, if we could get WordPress.org to change some of their infrastructure, something like this might make sense:

Switch the infrastructure from being based on plugins and themes (which all use the plugin or theme slug as the text domain) to being based on the text domains directly. For example, instead of https://api.wordpress.org/translations/plugins/1.0/ and https://api.wordpress.org/translations/themes/1.0/ just have one endpoint that takes the domain.
Let code declare to WordPress which domains it needs beyond the defaults of "plugin slug" and "theme slug". This is so WordPress can download those extra domains, I don't think WordPress cares beyond that.
Let us register projects that aren't plugins or themes (e.g. our packages) on translate.wordpress.org, so the packages can be translated and WordPress can fetch those translations like it does plugins and themes.

That way all we'd need to do would be to have the plugin declare the packages' text domains, and the translators would only have to translate each package's strings once instead of for every plugin using the package.

kbrown9 commented 3 years ago

A composer custom installer could put just the relevant packages outside of vendor/, assuming we're willing to identify them all with an appropriate "type" field in their composer.json files. That might avoid the trouble with the autoloader since all the metadata would still be in vendor/.

Your ideas seem reasonable to me.

Do you think there's any value in also investigating how the autoloader would need to be changed to allow composer to be configured to use something other than the default vendor directory? For example, I know that the autoloader will check for the generated classmap in the vendor directory, so we would need to make sure to keep the classmap in vendor.

anomiex commented 3 years ago

We could investigate it, but we shouldn't need to if my idea pans out. I suspect the best route if we did want to go in that direction would be to have it check for some specific non-vendor directory in addition to vendor/. Or, I guess, have the autoloader write vendor/ even if Composer uses something else.

kbrown9 commented 3 years ago

I suspect the best route if we did want to go in that direction would be to have it check for some specific non-vendor directory in addition to vendor/. Or, I guess, have the autoloader write vendor/ even if Composer uses something else.

If we did need to go in this direction, I think we would need to have the autoloader write some files to the vendor directory and have Composer use something else. This would be necessary for compatibility with the autoloader packages that are already in released plugins. Those autoloaders would be checking for the autoloader files in the other plugins' vendor directories. But, as you said, we probably won't need to go in this direction.

Automattic / jetpack