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.

Sign up for GitHub

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

Jump to bottom

Assure destination directory presence for localization strings files output #4478

Merged
merged 1 commit into from Jan 18, 2022
Merged

Assure destination directory presence for localization strings files output #4478

merged 1 commit into from Jan 18, 2022

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Jan 17, 2022 •
edited

This PR adds logic to both scripts in charge of generating the localization strings files (strings2android and strings2swift), in order to assure that the destination directory provided to the script is always present.

To test:

  1. Run command rm -rf bundle && npm run bundle.
  2. Observe that no errors are thrown and that bundles are generated.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot added the [Type] Enhancement Improves a current area of the editor label Jan 17, 2022
@fluiddot fluiddot requested a review from guarani January 17, 2022 15:09
@fluiddot fluiddot self-assigned this Jan 17, 2022
@fluiddot fluiddot changed the title Assure destination directory for localization strings files output Assure destination directory presence for localization strings files output Jan 17, 2022
@fluiddot fluiddot mentioned this pull request Jan 17, 2022
Merged
2 tasks
guarani
guarani reviewed Jan 18, 2022
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

I confirmed that running npm run bundle after having deleted the bundle results in a new bundle.

I see the following changes after the new bundle has been generated and I didn't expect to see changes because I was simply re-generating the bundle. Do you know why these changes occur?

	modified:   ios/App.js
	modified:   ios/App.js.map
	deleted:    ios/assets/gutenberg/packages/blocks/src/api/i18n-block.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ar.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/bg.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/bo.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ca.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/cs.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/cy.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/da.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/de.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/el.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/en-au.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/en-ca.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/en-gb.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/en-nz.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/en-za.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/es-ar.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/es-cl.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/es-cr.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/es.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/fa.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/fr.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/gl.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/he.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/hr.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/hu.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/id.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/is.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/it.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ja.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ka.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ko.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/nb.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/nl-be.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/nl.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/pl.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/pt-br.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/pt.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ro.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ru.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/sk.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/sq.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/sr.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/sv.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/th.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/tr.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/uk.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/ur.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/vi.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/zh-cn.json
	deleted:    ios/assets/gutenberg/packages/react-native-editor/i18n-cache/data/zh-tw.json
	deleted:    ios/assets/jetpack/projects/plugins/jetpack/extensions/index.json

@fluiddot
Copy link
Contributor Author

I see the following changes after the new bundle has been generated and I didn't expect to see changes because I was simply re-generating the bundle. Do you know why these changes occur?

Yep, this is expected after the merge of the RN 0.66 upgrade. Prior to 0.66 version, the JSON files referenced in the bundle were being duplicated by Metro upon generation as asset files, however, JSON files are part of the JS code of the bundle, so those files are redundant and not really used (bug reference). @dcalhoun opened this draft PR in order to address it, I don't think it's a high priority task but I could take over it if you agree.

@guarani guarani self-requested a review January 18, 2022 12:37
guarani
guarani approved these changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Yep, this is expected after the merge of the RN 0.66 upgrade. Prior to 0.66 version, the JSON files referenced in the bundle were being duplicated by Metro upon generation as asset files, however, JSON files are part of the JS code of the bundle, so those files are redundant and not really used (bug reference).

Ok, thanks for the explanation!

@dcalhoun opened this draft PR in order to address it, I don't think it's a high priority task but I could take over it if you agree.

Thanks for pointing out that PR. I agree it doesn't seem high-priority either 👍

@guarani
Copy link
Contributor

guarani commented Jan 18, 2022

I left an approval 👍 Thanks @fluiddot!

@peril-wordpress-mobile
Copy link

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@fluiddot fluiddot merged commit b9eb1e4 into develop Jan 18, 2022
@fluiddot fluiddot deleted the fix/localization-strings-file-destination-dir branch January 18, 2022 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guarani guarani guarani approved these changes

Assignees

@fluiddot fluiddot

Labels
[Type] Enhancement Improves a current area of the editor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@fluiddot @guarani