Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

refactor: migrate to react-intl from react-i18next #240

Merged
merged 20 commits into from Jun 10, 2022

Conversation

keiya01
Copy link
Member

@keiya01 keiya01 commented Jun 4, 2022

Overview

What I've done

I replaced react-intl with react-i18next.
And I removed react-intl pkg.
closes reearth/reearth#172.

What I haven't done

How I tested

  1. Access to reearth staging environment
  2. Check all of text

Screenshot

Which point I want you to review particularly

  • I replaced useIntl with useT.
  • I replaced ja translation file by using the following script.
replacing script

import fs from "node:fs/promises";

import yaml from "js-yaml";

const findKeyByValueRecursiveObj = (obj, val) => {
const keys = Object.keys(obj);
for (const key of keys) {
const objValue = obj[key];
if (typeof objValue === "string") {
if (objValue === val) return key;
continue;
}
const foundKey = findKeyByValue(objValue, val);
if (foundKey) {
return foundKey;
}
}
};

const findValueByKeyRecursiveObj = (obj, key) => {
const keys = Object.keys(obj);
for (const objKey of keys) {
const objValue = obj[objKey];
if (typeof objValue === "string") {
if (objKey === key) return objValue;
continue;
}
const foundValue = findValueByKey(objValue, key);
if (foundValue) {
return foundValue;
}
}
};

const findKeyByValue = (obj, val) => {
if (Array.isArray(obj)) {
throw Error("Array is not supported");
} else {
return findKeyByValueRecursiveObj(obj, val);
}
};

const findValueByKey = (obj, key) => {
if (Array.isArray(obj)) {
throw Error("Array is not supported");
} else {
return findValueByKeyRecursiveObj(obj, key);
}
};

// You need to run this script after yarn i18n.
const run = async () => {
const [legacyEnRaw, legacyJaRaw, newJaRaw] = await Promise.all([
fs.readFile("./src/i18n/translations/legacy/en.yml", "utf8"),
fs.readFile("./src/i18n/translations/legacy/ja.yml", "utf8"),
fs.readFile("./src/i18n/translations/ja.yml", "utf8"),
]);
const legacyEn = yaml.load(legacyEnRaw);
const legacyJa = yaml.load(legacyJaRaw);
const newJa = yaml.load(newJaRaw);

const result = {};
const keys = Object.keys(newJa);
for (const key of keys) {
// If value is only exist in new ja file then use existing value.
if (newJa[key]) {
result[key] = newJa[key];
continue;
}

const replacedKey = key.replace(/\{\{(.+)\}\}/, "{$1}");
const hash = findKeyByValue(legacyEn, replacedKey);
if (hash) {
  const value = findValueByKey(legacyJa, hash);
  result[key] = value;
} else {
  throw new Error("Hash is not found");
}

}

const rawYaml = yaml.dump(result);
await fs.writeFile("./src/i18n/translations/ja.yml", rawYaml, "utf8");
};

run();

Memo

@keiya01 keiya01 self-assigned this Jun 4, 2022
@netlify
Copy link

netlify bot commented Jun 4, 2022

Deploy Preview for reearth-web ready!

Name Link
🔨 Latest commit 410e414
🔍 Latest deploy log https://app.netlify.com/sites/reearth-web/deploys/62a1d7ecf3253300087ebafb
😎 Deploy Preview https://deploy-preview-240--reearth-web.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Jun 4, 2022

Codecov Report

Merging #240 (410e414) into main (f980054) will decrease coverage by 0.24%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
- Coverage   51.48%   51.24%   -0.25%     
==========================================
  Files          58       56       -2     
  Lines        1175     1163      -12     
  Branches      184      182       -2     
==========================================
- Hits          605      596       -9     
+ Misses        506      505       -1     
+ Partials       64       62       -2     
Impacted Files Coverage Δ
src/i18n/i18n.ts 100.00% <ø> (ø)
src/i18n/index.tsx 100.00% <ø> (ø)
src/i18n/publishedProvider.tsx 0.00% <0.00%> (ø)
src/i18n/locale.ts 100.00% <100.00%> (ø)
src/i18n/provider.tsx 83.33% <100.00%> (ø)

@keiya01
Copy link
Member Author

keiya01 commented Jun 4, 2022

fix: use key as english translation
I think we can use translation object key as translation text in english.
For example, when we use useT("Hello World") then translation key is Hello World. And I think in most case, this key is actual text used in web page.
Therefore we don't need to define en.yml translation file, we can use key as translation text in english.
When user specify ja lang, service text is displayed in Japanese.
Do @rot1024 @KaWaite think about this?

@keiya01 keiya01 force-pushed the feature/replace-react-intl branch from b83c3a5 to 0269772 Compare June 4, 2022 13:31
Copy link
Member

@rot1024 rot1024 left a comment

Choose a reason for hiding this comment

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

  • Delete i18n:legacy script in package.json also.
  • Resolve conflict, but It would not be a problem to override change in this PR.
  • CI failed. CI is set up to report an error if there are missing translation. Rerun yarn i18n

package.json Outdated Show resolved Hide resolved
replace_i18n.mjs Outdated Show resolved Hide resolved
i18next-parser.config.js Outdated Show resolved Hide resolved
@rot1024 rot1024 changed the title refactor: replace react-intl with react-i18next refactor: migrate to react-intl from react-i18next Jun 7, 2022
@keiya01 keiya01 enabled auto-merge (squash) June 7, 2022 11:02
src/i18n/i18n.ts Outdated Show resolved Hide resolved
Copy link
Member

@KaWaite KaWaite left a comment

Choose a reason for hiding this comment

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

Looked over the config files only, but everything looks good! I asked for a single change, so please do that before merging.

Also, I'll approve, but can you please test storybook locally? The PR's preview deploy on netlify failed. Sometimes changes to yarn.lock, especially deleting yarn.lock and regenerating, will break storybook....
If it fails on local as well, please investigate, fix and ask for a re-review! If it works locally it might be an issue with redeployment on netfliy and should be okay for us.

@keiya01 keiya01 merged commit 4047436 into main Jun 10, 2022
@keiya01 keiya01 deleted the feature/replace-react-intl branch June 10, 2022 03:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants