Fix: conflicting strings issue in translations (#917)

* Fix conflicting strings issue in translations * Preserve structure of `customTranslations` * Use `deepmerge` to merge whole of `localized-strings` * Simplify and make deep property access on an object safe * Fix deep property accessor and rename it to idx
facebook · Aug 28, 2018 · cfabaed · cfabaed
1 parent d18b099
commit cfabaed
Show file tree

Hide file tree

Showing 12 changed files with 94 additions and 66 deletions.
diff --git a/docs/guides-translation.md b/docs/guides-translation.md
@@ -100,8 +100,16 @@ If you want to add additional custom translation strings, or override any of the
 ```json
 {
   "localized-strings": {
-    "id": "string",
-    "id2": "string2"
+    "docs": {
+      "id": {
+        "title": "string1",
+        "sidebar_label": "string2"
+      },
+      "version-0.0.1-id": {
+        "title": "string3",
+        "sidebar_label": "string4"
+      }
+    }
   },
   "pages-strings" : {
     "id3": "string3",

diff --git a/lib/core/DocsLayout.js b/lib/core/DocsLayout.js
@@ -16,6 +16,7 @@ const DocsSidebar = require('./DocsSidebar.js');
 const OnPageNav = require('./nav/OnPageNav.js');
 const Site = require('./Site.js');
 const translation = require('../server/translation.js');
+const {idx} = require('./utils.js');
 
 // component used to generate whole webpage for docs, including sidebar/header/footer
 class DocsLayout extends React.Component {
@@ -35,16 +36,15 @@ class DocsLayout extends React.Component {
   render() {
     const metadata = this.props.metadata;
     const content = this.props.children;
-    const i18n = translation[this.props.metadata.language];
+    const i18n = translation[metadata.language];
+    const id = metadata.localized_id;
+    const defaultTitle = metadata.title;
     let DocComponent = Doc;
     if (this.props.Doc) {
       DocComponent = this.props.Doc;
     }
-    const title = i18n
-      ? translation[this.props.metadata.language]['localized-strings'][
-          this.props.metadata.localized_id
-        ] || this.props.metadata.title
-      : this.props.metadata.title;
+    const title =
+      idx(i18n, ['localized-strings', 'docs', id, 'title']) || defaultTitle;
     const hasOnPageNav = this.props.config.onPageNav === 'separate';
     return (
       <Site
@@ -65,7 +65,7 @@ class DocsLayout extends React.Component {
               content={content}
               config={this.props.config}
               source={metadata.source}
-              hideTitle={this.props.metadata.hide_title}
+              hideTitle={metadata.hide_title}
               title={title}
               version={metadata.version}
               language={metadata.language}
@@ -79,15 +79,10 @@ class DocsLayout extends React.Component {
                     metadata.previous_id
                   )}>
                   ←{' '}
-                  {i18n
-                    ? translation[this.props.metadata.language][
-                        'localized-strings'
-                      ][metadata.previous_id] ||
-                      translation[this.props.metadata.language][
-                        'localized-strings'
-                      ].previous ||
-                      'Previous'
-                    : metadata.previous_title || 'Previous'}
+                  {idx(i18n, ['localized-strings', metadata.previous_id]) ||
+                    idx(i18n, ['localized-strings', 'previous']) ||
+                    metadata.previous_title ||
+                    'Previous'}
                 </a>
               )}
               {metadata.next_id && (
@@ -97,23 +92,18 @@ class DocsLayout extends React.Component {
                     metadata.localized_id,
                     metadata.next_id
                   )}>
-                  {i18n
-                    ? translation[this.props.metadata.language][
-                        'localized-strings'
-                      ][metadata.next_id] ||
-                      translation[this.props.metadata.language][
-                        'localized-strings'
-                      ].next ||
-                      'Next'
-                    : metadata.next_title || 'Next'}{' '}
+                  {idx(i18n, ['localized-strings', metadata.next_id]) ||
+                    idx(i18n, ['localized-strings', 'next']) ||
+                    metadata.next_title ||
+                    'Next'}{' '}
                   →
                 </a>
               )}
             </div>
           </Container>
           {hasOnPageNav && (
             <nav className="onPageNav">
-              <OnPageNav rawContent={this.props.children} />
+              <OnPageNav rawContent={content} />
             </nav>
           )}
         </div>

diff --git a/lib/core/Redirect.js b/lib/core/Redirect.js
@@ -8,13 +8,14 @@
 const React = require('react');
 const Head = require('./Head.js');
 const translation = require('../server/translation.js');
+const {idx} = require('./utils.js');
 
 // Component used to provide same head, header, footer, other scripts to all pages
 class Redirect extends React.Component {
   render() {
-    const tagline = translation[this.props.language]
-      ? translation[this.props.language]['localized-strings'].tagline
-      : this.props.config.tagline;
+    const tagline =
+      idx(translation, [this.props.language, 'localized-strings', 'tagline']) ||
+      this.props.config.tagline;
     const title = this.props.title
       ? `${this.props.title} · ${this.props.config.title}`
       : (!this.props.config.disableTitleTagline &&

diff --git a/lib/core/Site.js b/lib/core/Site.js
@@ -14,15 +14,16 @@ const Head = require('./Head.js');
 const Footer = require(`${process.cwd()}/core/Footer.js`);
 const translation = require('../server/translation.js');
 const constants = require('./constants');
+const {idx} = require('./utils.js');
 
 const CWD = process.cwd();
 
 // Component used to provide same head, header, footer, other scripts to all pages
 class Site extends React.Component {
   render() {
-    const tagline = translation[this.props.language]
-      ? translation[this.props.language]['localized-strings'].tagline
-      : this.props.config.tagline;
+    const tagline =
+      idx(translation, [this.props.language, 'localized-strings', 'tagline']) ||
+      this.props.config.tagline;
     const title = this.props.title
       ? `${this.props.title} · ${this.props.config.title}`
       : (!this.props.config.disableTitleTagline &&

diff --git a/lib/core/nav/HeaderNav.js b/lib/core/nav/HeaderNav.js
@@ -22,7 +22,7 @@ const readMetadata = require('../../server/readMetadata.js');
 
 readMetadata.generateMetadataDocs();
 const Metadata = require('../metadata.js');
-const utils = require('../utils.js');
+const {idx, getPath} = require('../utils.js');
 
 const extension = siteConfig.cleanUrl ? '' : '.html';
 
@@ -56,7 +56,7 @@ class LanguageDropDown extends React.Component {
         }
         return (
           <li key={lang.tag}>
-            <a href={utils.getPath(href, this.props.cleanUrl)}>{lang.name}</a>
+            <a href={getPath(href, this.props.cleanUrl)}>{lang.name}</a>
           </li>
         );
       });
@@ -188,7 +188,7 @@ class HeaderNav extends React.Component {
       }
       href =
         this.props.config.baseUrl +
-        utils.getPath(Metadata[id].permalink, this.props.config.cleanUrl);
+        getPath(Metadata[id].permalink, this.props.config.cleanUrl);
 
       const {id: currentID, sidebar} = this.props.current;
       docItemActive = currentID && currentID === id;
@@ -220,12 +220,11 @@ class HeaderNav extends React.Component {
         (link.blog && this.props.current.blogListing) ||
         (link.page && link.page === this.props.current.id),
     });
+    const i18n = translation[this.props.language];
     return (
       <li key={`${link.label}page`} className={itemClasses}>
         <a href={href} target={link.external ? '_blank' : '_self'}>
-          {translation[this.props.language]
-            ? translation[this.props.language]['localized-strings'][link.label]
-            : link.label}
+          {idx(i18n, ['localized-strings', 'links', link.label]) || link.label}
         </a>
       </li>
     );

diff --git a/lib/core/nav/SideNav.js b/lib/core/nav/SideNav.js
@@ -10,48 +10,50 @@ const classNames = require('classnames');
 
 const siteConfig = require(`${process.cwd()}/siteConfig.js`);
 const translation = require('../../server/translation.js');
-const utils = require('../utils.js');
+const {getPath, idx} = require('../utils.js');
 
 class SideNav extends React.Component {
   // return appropriately translated category string
   getLocalizedCategoryString(category) {
-    const categoryString = translation[this.props.language]
-      ? translation[this.props.language]['localized-strings'][category] ||
-        category
-      : category;
+    const categoryString =
+      idx(translation, [
+        this.props.language,
+        'localized-strings',
+        'categories',
+        category,
+      ]) || category;
     return categoryString;
   }
 
   // return appropriately translated label to use for doc/blog in sidebar
   getLocalizedString(metadata) {
     let localizedString;
     const i18n = translation[this.props.language];
+    const id = metadata.localized_id;
     const sbTitle = metadata.sidebar_label;
 
     if (sbTitle) {
-      localizedString = i18n
-        ? i18n['localized-strings'][sbTitle] || sbTitle
-        : sbTitle;
+      localizedString =
+        idx(i18n, ['localized-strings', 'docs', id, 'sidebar_label']) ||
+        sbTitle;
     } else {
-      const id = metadata.original_id || metadata.localized_id;
-      localizedString = i18n
-        ? i18n['localized-strings'][id] || metadata.title
-        : metadata.title;
+      localizedString =
+        idx(i18n, ['localized-strings', 'docs', id, 'title']) || metadata.title;
     }
     return localizedString;
   }
 
   // return link to doc in sidebar
   getLink(metadata) {
     if (metadata.permalink) {
-      const targetLink = utils.getPath(metadata.permalink, siteConfig.cleanUrl);
+      const targetLink = getPath(metadata.permalink, siteConfig.cleanUrl);
       if (targetLink.match(/^https?:/)) {
         return targetLink;
       }
       return siteConfig.baseUrl + targetLink;
     }
     if (metadata.path) {
-      return `${siteConfig.baseUrl}blog/${utils.getPath(
+      return `${siteConfig.baseUrl}blog/${getPath(
         metadata.path,
         siteConfig.cleanUrl
       )}`;

diff --git a/lib/core/utils.js b/lib/core/utils.js
@@ -28,9 +28,14 @@ function getPath(pathStr, cleanUrl = false) {
     : removeExtension(pathStr);
 }
 
+function idx(target, path) {
+  return path.reduce((obj, key) => obj && obj[key], target);
+}
+
 module.exports = {
   blogPostHasTruncateMarker,
   extractBlogPostBeforeTruncate,
   getPath,
   removeExtension,
+  idx,
 };
diff --git a/lib/server/readMetadata.js b/lib/server/readMetadata.js
@@ -180,7 +180,7 @@ function generateMetadataDocs() {
 
   // metadata for english files
   const docsDir = path.join(CWD, '../', getDocsPath());
-  let files = glob.sync(`${CWD}/../${getDocsPath()}/**`);
+  let files = glob.sync(`${docsDir}/**`);
   files.forEach(file => {
     const extension = path.extname(file);
 

diff --git a/lib/write-translations.js b/lib/write-translations.js
@@ -26,6 +26,7 @@ const fs = require('fs-extra');
 const glob = require('glob');
 const mkdirp = require('mkdirp');
 const nodePath = require('path');
+const deepmerge = require('deepmerge');
 
 const readMetadata = require('./server/readMetadata.js');
 
@@ -34,12 +35,19 @@ const siteConfig = require(`${CWD}/siteConfig.js`);
 const sidebars = require(`${CWD}/sidebars.json`);
 
 let customTranslations = {
-  'localized-strings': {},
+  'localized-strings': {
+    docs: {},
+    links: {},
+    categories: {},
+  },
   'pages-strings': {},
 };
 if (fs.existsSync(`${CWD}/data/custom-translation-strings.json`)) {
-  customTranslations = JSON.parse(
-    fs.readFileSync(`${CWD}/data/custom-translation-strings.json`, 'utf8')
+  customTranslations = deepmerge(
+    JSON.parse(
+      fs.readFileSync(`${CWD}/data/custom-translation-strings.json`, 'utf8')
+    ),
+    customTranslations
   );
 }
 
@@ -54,13 +62,20 @@ function execute() {
       next: 'Next',
       previous: 'Previous',
       tagline: siteConfig.tagline,
+      docs: {},
+      links: {},
+      categories: {},
     },
     'pages-strings': {},
   };
 
   // look through markdown headers of docs for titles and categories to translate
   const docsDir = nodePath.join(CWD, '../', readMetadata.getDocsPath());
-  let files = glob.sync(`${CWD}/../${readMetadata.getDocsPath()}/**`);
+  const versionedDocsDir = nodePath.join(CWD, 'versioned_docs');
+  let files = [
+    ...glob.sync(`${docsDir}/**`),
+    ...glob.sync(`${versionedDocsDir}/**`),
+  ];
   files.forEach(file => {
     const extension = nodePath.extname(file);
     if (extension === '.md' || extension === '.markdown') {
@@ -75,27 +90,29 @@ function execute() {
         return;
       }
       const metadata = res.metadata;
+      const id = metadata.localized_id;
 
-      translations['localized-strings'][metadata.localized_id] = metadata.title;
+      translations['localized-strings'].docs[id] = {};
+      translations['localized-strings'].docs[id].title = metadata.title;
 
       if (metadata.sidebar_label) {
-        translations['localized-strings'][metadata.sidebar_label] =
+        translations['localized-strings'].docs[id].sidebar_label =
           metadata.sidebar_label;
       }
     }
   });
   // look through header links for text to translate
   siteConfig.headerLinks.forEach(link => {
     if (link.label) {
-      translations['localized-strings'][link.label] = link.label;
+      translations['localized-strings'].links[link.label] = link.label;
     }
   });
 
   // find sidebar category titles to translate
   Object.keys(sidebars).forEach(sb => {
     const categories = sidebars[sb];
     Object.keys(categories).forEach(category => {
-      translations['localized-strings'][category] = category;
+      translations['localized-strings'].categories[category] = category;
     });
   });
 
@@ -120,7 +137,7 @@ function execute() {
     Object.keys(sidebarContent).forEach(sb => {
       const categories = sidebarContent[sb];
       Object.keys(categories).forEach(category => {
-        translations['localized-strings'][category] = category;
+        translations['localized-strings'].categories[category] = category;
       });
     });
   });
@@ -170,7 +187,7 @@ function execute() {
     translations['pages-strings'],
     customTranslations['pages-strings']
   );
-  translations['localized-strings'] = Object.assign(
+  translations['localized-strings'] = deepmerge(
     translations['localized-strings'],
     customTranslations['localized-strings']
   );

diff --git a/package.json b/package.json
@@ -70,6 +70,7 @@
     "commander": "^2.16.0",
     "crowdin-cli": "^0.3.0",
     "cssnano": "^3.10.0",
+    "deepmerge": "^2.1.1",
     "escape-string-regexp": "^1.0.5",
     "express": "^4.15.3",
     "feed": "^1.1.0",

diff --git a/website/data/custom-translation-strings.json b/website/data/custom-translation-strings.json
@@ -4,6 +4,6 @@
     "translation": "Translations and Localization"
   },
   "pages-strings" : {
-        "Help Translate|recruit community translators for your project": "Help Us Translate"
+    "Help Translate|recruit community translators for your project": "Help Us Translate"
   }
 }