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

Localized strings files are missing strings and adding extra ones #3466

Closed
fluiddot opened this issue May 7, 2021 · 4 comments
Closed

Localized strings files are missing strings and adding extra ones #3466

fluiddot opened this issue May 7, 2021 · 4 comments
Labels
release-process [Type] Bug Something isn't working

Comments

@fluiddot
Copy link
Contributor

fluiddot commented May 7, 2021

Describe the bug
For each platform we generate a specific file with the localized strings:

I investigated further the process of generating these files and I noticed the following issues:

Missing strings

Comparing the files generated in the latest releases, I saw that some strings were missing, for example, the string Block copied is present in version 1.51.1 but not in 1.52.0, here is the diff of the commit that introduced the change.

I debugged the scripts that we use for extracting the strings from the codebase and looks like that some files are not including their strings. Specifically, when we run the command with the debug option (npm run makepot:ios -- --debug), we can see that some files can't be parsed (see attached logs). This is caused due to the use of the optional chaining operator that is not supported by the script (this is known bug) we use to extract the strings (wp i18n make-pot).

Parse file errors generated by npm run makepot:ios
Debug: Could not parse file block-editor/src/components/block-icon/index.native.js: Unexpected: . (line 23, column 11) (2.163s)
Debug: Could not parse file block-editor/src/components/block-list/block-list-item.native.js: Unexpected: . (line 211, column 16) (2.18s)
Debug: Could not parse file block-editor/src/components/block-list/block-selection-button.native.js: Unexpected: . (line 63, column 29) (2.184s)
Debug: Could not parse file block-editor/src/components/block-mobile-toolbar/block-actions-menu.native.js: Unexpected: . (line 194, column 20) (2.31s)
Debug: Could not parse file block-editor/src/components/block-mobile-toolbar/index.native.js: Unclosed { (line 106, column 0) (2.316s)
Debug: Could not parse file block-editor/src/components/block-switcher/block-transformations-menu.native.js: Unexpected: . (line 32, column 42) (2.363s)
Debug: Could not parse file block-editor/src/components/inserter/menu.native.js: Unexpected: . (line 102, column 18) (2.436s)
Debug: Could not parse file block-library/src/button/edit.native.js: Unexpected: . (line 47, column 21) (2.597s)
Debug: Could not parse file block-library/src/column/edit.native.js: Unexpected: . (line 255, column 4) (2.632s)
Debug: Could not parse file block-library/src/columns/columnCalculations.native.js: Unexpected: . (line 27, column 42) (2.633s)
Debug: Could not parse file block-library/src/columns/edit.native.js: Unexpected: . (line 476, column 61) (2.659s)
Debug: Could not parse file block-library/src/cover/controls.native.js: Unexpected: . (line 137, column 32) (2.667s)
Debug: Could not parse file block-library/src/cover/edit.native.js: Unexpected: . (line 252, column 17) (2.679s)
Debug: Could not parse file block-library/src/cover/overlay-color-settings.native.js: Unexpected: . (line 57, column 20) (2.685s)
Debug: Could not parse file block-library/src/group/edit.native.js: Unexpected: . (line 86, column 17) (2.793s)
Debug: Could not parse file block-library/src/media-text/edit.native.js: Unexpected: . (line 299, column 17) (2.891s)
Debug: Could not parse file block-library/src/search/edit.native.js: Unexpected: . (line 37, column 20) (2.974s)
Debug: Could not parse file block-library/src/social-link/edit.native.js: Unexpected: . (line 211, column 21) (3.005s)
Debug: Could not parse file block-library/src/social-links/edit.native.js: Unexpected: . (line 129, column 33) (3.012s)
Debug: Could not parse file components/src/focal-point-picker/index.native.js: Unexpected: . (line 112, column 34) (3.134s)
Debug: Could not parse file components/src/focal-point-picker/tooltip/index.native.js: Unexpected: . (line 98, column 37) (3.139s)
Debug: Could not parse file components/src/higher-order/with-focus-outside/index.native.js: Unexpected: . (line 20, column 10) (3.141s)
Debug: Could not parse file components/src/mobile/bottom-sheet-select-control/index.native.js: Unexpected: > (line 70, column 4) (3.168s)
Debug: Could not parse file components/src/mobile/bottom-sheet-text-control/index.native.js: Unexpected: > (line 72, column 4) (3.172s)
Debug: Could not parse file components/src/mobile/bottom-sheet/index.native.js: Unexpected: > (line 466, column 10) (3.249s)
Debug: Could not parse file components/src/mobile/bottom-sheet/navigation-header.native.js: Unexpected: > (line 81, column 6) (3.267s)
Debug: Could not parse file components/src/mobile/color-settings/utils.native.js: Unexpected: . (line 12, column 12) (3.364s)
Debug: Could not parse file components/src/mobile/global-styles-context/utils.native.js: Unexpected: . (line 24, column 22) (3.382s)
Debug: Could not parse file components/src/mobile/gradient/index.native.js: Unexpected: . (line 70, column 51) (3.386s)
Debug: Could not parse file components/src/mobile/image/index.native.js: Unexpected: . (line 83, column 19) (3.426s)
Debug: Could not parse file components/src/mobile/media-edit/index.native.js: Unexpected: . (line 51, column 10) (3.494s)
Debug: Could not parse file components/src/mobile/utils/use-unit-converter-to-mobile.native.js: Unexpected: . (line 26, column 3) (3.526s)
Debug: Could not parse file components/src/notice/index.native.js: Unexpected: . (line 48, column 23) (3.529s)
Debug: Could not parse file components/src/unit-control/index.native.js: Unexpected: . (line 48, column 17) (3.581s)
Debug: Could not parse file format-library/src/link/modal-screens/link-settings-screen.native.js: Unexpected: . (line 174, column 31) (3.73s)
Debug: Could not parse file primitives/src/block-quotation/index.native.js: Unexpected: . (line 23, column 8) (3.737s)
Debug: Could not parse file rich-text/src/component/index.native.js: Unexpected: . (line 924, column 32) (3.808s)
Debug: Could not parse file react-native-editor/bundle/ios/App.js.map: Unexpected: ErrorHandler (line 1457, column 5 in the concatenated sourcesContent) (6.497s)

Solution

Until the issue gets fixed, the only solution I see is to stop using the optional chaining operator, at least on files where we have localized strings 😞 . Another approach would be to add the strings generated by those files manually (there're 38 files affected by this).

As an alternative, I tried to use a babel plugin that we have in the Gutenberg repo (@wordpress/babel-plugin-makepot) for this but unfortunately, it's not including all the strings.

I saw that in the web version, we use optional chaining in the code and since the translations are being generated properly, I'm wondering what they do differently, so I'll investigate further if we could replicate partially that process 🤔

Extra strings

Similar to the previous issue, comparing the generated files from previous releases, I noticed that we recently added a lot of strings that are not even being used in the native version, here you can check that we introduced 538 new strings 😲 .

Before generating the localized string files, we make an intermediate POT file, I checked manually this file and I found that some strings are extracted from files that aren't *.native.js files, even including PHP files 😮 .

As per the documentation of wp i18n make-pot, the argument --include should only include the specified files, however, this is not working as expected and I found out that is a known bug.

Solution

We could explicitly exclude the files that we don't want to include through the --exclude argument, in this case, we could add *.js and *.php.

The commands will result in the following:

"makepot:android": "npm run wp -- i18n make-pot ./gutenberg/packages --include=*.native.js,*.android.js --exclude=test/*,e2e-tests/*,*.php,*.js --subtract=./gutenberg.pot --ignore-domain gutenberg-android.pot",
"makepot:ios": "npm run wp -- i18n make-pot ./gutenberg/packages --include=*.native.js,*.ios.js --exclude=test/*,e2e-tests/*,*.php,*.js --subtract=./gutenberg.pot --ignore-domain gutenberg-ios.pot",

To Reproduce

  1. Run the command npm run genstrings
  2. Check localized strings file (iOS: GutenbergNativeTranslations.swift and Android: strings.xml) and look for extra/missing strings.

Expected behavior
The localized strings file should only contain the strings extracted from *.native.js files.

@swissspidy
Copy link

This is caused due to the use of the optional chaining operator that is not supported by the script (this is known bug)

Is this still an issue for you? The underlying parser was updated a while ago, so I'd expect this to be fixed in WP-CLI 2.5

@fluiddot
Copy link
Contributor Author

fluiddot commented Jul 5, 2021

This is caused due to the use of the optional chaining operator that is not supported by the script (this is known bug)

Is this still an issue for you? The underlying parser was updated a while ago, so I'd expect this to be fixed in WP-CLI 2.5

Most likely not because we actually forcing use the last version of WP-CLI (2.5) on this command but let me do a quick check before confirming it.

@fluiddot
Copy link
Contributor Author

fluiddot commented Jul 5, 2021

I've just verified that the issue is no longer happening 🎊 , at least with the chain operator but I noticed that the parsing fails on files that contain the fragment short syntax <></>. I'll open a new issue with this case, so we can close this one as it's already fixed.

Missing strings

This issue has been fixed by using the last version of WP-CLI that was incorporated in this PR.

Extra strings

This issue has been fixed in this PR.

As a side note, we're still using the cached version of the Gutenberg localizations that was fetched for version 1.52.2. I checked that the translations from GlotPress already include the missing strings so now we can start updating the cache when cutting a new release (I'll open another issue as a follow-up for this).

@fluiddot
Copy link
Contributor Author

@swissspidy Heads-up that I opened a new issue because looks like we still have a high number of missing translation after the fixes:
WordPress/gutenberg#33364

SiobhyB pushed a commit that referenced this issue Nov 25, 2021
For further details on bug that requires this manual change, see: #3466
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-process [Type] Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants