Fix always-English data path separators (#63504)

Co-authored-by: Hector A. <v-halfaro@github.com>
Copilot-Session: 7c52b57f-2c29-4aa1-8233-99d0d6e13571

Author: heiskr

src/data-directory/lib/get-data.ts

@@ -43,28 +43,34 @@ export const getDeepDataByLanguage = memoize(
     if (dir === null) {
       dir = languages[langCode].dir
     }
-    return getDeepDataByDir(dottedPath, dir)
+    const englishRoot = langCode === 'en' ? dir : languages.en.dir
+    return getDeepDataByDir(dottedPath, dir, englishRoot)
   },
 )
 
-// getDeepDataByLanguage caches each top-level path, so recursive reads need no extra cache.
-function getDeepDataByDir(dottedPath: string, dir: string): Record<string, unknown> {
+// Doesn't need to be memoized because it's used by getDataKeysByLanguage
+// which is already memoized.
+function getDeepDataByDir(
+  dottedPath: string,
+  dir: string,
+  englishRoot: string,
+): Record<string, unknown> {
   const fullPath = ['data']
   const split = dottedPath.split(/\./g)
   fullPath.push(...split)
 
   const things: Record<string, unknown> = {}
-  const relPath = fullPath.join(path.sep)
+  const relPath = path.posix.join(...fullPath)
   for (const dirent of getDirents(dir, relPath)) {
     if (dirent.name === 'README.md') continue
     // Release-note basenames like '3-5' and '0-rc2' stay intact.
     const key = dirent.isDirectory() ? dirent.name : dirent.name.replace(/\.yml$/, '')
     if (dirent.isDirectory()) {
-      things[key] = getDeepDataByDir(`${dottedPath}.${key}`, dir)
+      things[key] = getDeepDataByDir(`${dottedPath}.${key}`, dir, englishRoot)
     } else if (dirent.name.endsWith('.yml')) {
-      things[key] = getYamlContent(dir, path.join(relPath, dirent.name))
+      things[key] = getYamlContent(dir, path.posix.join(relPath, dirent.name), englishRoot)
     } else if (dirent.name.endsWith('.md')) {
-      things[key] = getMarkdownContent(dir, path.join(relPath, dirent.name))
+      things[key] = getMarkdownContent(dir, path.posix.join(relPath, dirent.name), englishRoot)
     } else {
       throw new Error(`don't know how to read '${dirent.name}'`)
     }
@@ -91,7 +97,7 @@ export const getUIDataMerged = memoize((langCode: string): UIStrings => {
 const getUIData = (langCode: string): Record<string, unknown> => {
   const fullPath = ['data', 'ui.yml']
   const { dir } = languages[langCode]
-  return getYamlContent(dir, fullPath.join(path.sep)) as Record<string, unknown>
+  return getYamlContent(dir, path.posix.join(...fullPath)) as Record<string, unknown>
 }
 
 // When translated data misses a dotted path, retry English.
@@ -105,7 +111,7 @@ export const getDataByLanguage = memoize((dottedPath: string, langCode: string):
     const value = getDataByDir(dottedPath, dir, languages.en.dir, langCode)
 
     if (value === undefined && langCode !== 'en') {
-      return getDataByDir(dottedPath, languages.en.dir)
+      return getDataByDir(dottedPath, languages.en.dir, languages.en.dir)
     }
     return value
   } catch (error) {
@@ -115,7 +121,8 @@ export const getDataByLanguage = memoize((dottedPath: string, langCode: string):
         if (DEBUG_JIT_DATA_READS) {
           logger.warn('Unable to parse Yaml in translation', { langCode, dottedPath, error })
         }
-        return getDataByDir(dottedPath, languages.en.dir)
+        // Give it one more chance, but use English this time
+        return getDataByDir(dottedPath, languages.en.dir, languages.en.dir)
       }
       // Throw English YAML errors so staff writers see corrupt source data early.
       throw error
@@ -152,21 +159,18 @@ function getDataByDir(
     const basename = split.pop()!
     fullPath.push(...split)
     fullPath.push(`${basename}.yml`)
-    const allData = getYamlContent(dir, fullPath.join(path.sep), englishRoot) as
-      | Record<string, unknown>
-      | undefined
+    const relPath = path.posix.join(...fullPath)
+    const allData = getYamlContent(dir, relPath, englishRoot) as Record<string, unknown> | undefined
     if (allData && key) {
       const value = allData[key]
       if (value) {
         let content = matter(value as string).content
         if (dir !== englishRoot) {
           let englishContent = content
           try {
-            const englishData = getYamlContent(
-              englishRoot,
-              fullPath.join(path.sep),
-              englishRoot,
-            ) as Record<string, unknown> | undefined
+            const englishData = getYamlContent(englishRoot, relPath, englishRoot) as
+              | Record<string, unknown>
+              | undefined
             if (englishData?.[key]) {
               englishContent = matter(englishData[key] as string).content
             }
@@ -183,7 +187,7 @@ function getDataByDir(
         return content
       }
     } else {
-      logger.warn('Unable to find variables Yaml file', { filePath: fullPath.join(path.sep) })
+      logger.warn('Unable to find variables Yaml file', { filePath: relPath })
     }
     return undefined
   }
@@ -192,13 +196,14 @@ function getDataByDir(
     const nakedname = split.pop()!
     fullPath.push(...split)
     fullPath.push(`${nakedname}.md`)
-    const markdown = getMarkdownContent(dir, fullPath.join(path.sep), englishRoot)
+    const relPath = path.posix.join(...fullPath)
+    const markdown = getMarkdownContent(dir, relPath, englishRoot)
     let { content } = matter(markdown)
     if (dir !== englishRoot) {
       // Translated reusables need English content to fix corruptions like [AUTOTITLE"을](/foo/bar).
       let englishContent = content
       try {
-        englishContent = getMarkdownContent(englishRoot, fullPath.join(path.sep), englishRoot)
+        englishContent = getMarkdownContent(englishRoot, relPath, englishRoot)
       } catch (error) {
         // Translated pages can reference reusables missing in English; other corrections still run.
         if ((error as FileSystemError).code !== 'ENOENT') {
@@ -217,15 +222,15 @@ function getDataByDir(
   if (first === 'ui') {
     const basename = split.shift()
     fullPath.push(`${basename}.yml`)
-    const allData = getYamlContent(dir, fullPath.join(path.sep), englishRoot)
+    const allData = getYamlContent(dir, path.posix.join(...fullPath), englishRoot)
     return get(allData, split.join('.'))
   }
 
   if (first === 'glossaries' || first === 'release-notes') {
     const basename = split.pop()!
     fullPath.push(...split)
     fullPath.push(`${basename}.yml`)
-    return getYamlContent(dir, fullPath.join(path.sep), englishRoot)
+    return getYamlContent(dir, path.posix.join(...fullPath), englishRoot)
   }
 
   throw new Error(`Can't find the key '${dottedPath}' in the scope.`)

src/data-directory/tests/get-data.ts

@@ -28,6 +28,12 @@ describe('get-data', () => {
           },
         },
         variables: {
+          copilot: {
+            prodname_copilot: 'GitHub Copilot',
+          },
+          product: {
+            company_short: 'GitHub',
+          },
           stuff: {
             foo: 'Foo',
             bar: 'Bar',
@@ -36,6 +42,10 @@ describe('get-data', () => {
         reusables: {
           coolness: 'This is *Markdown*',
           otherness: '**Also** Markdown',
+          ssh: {
+            fingerprints: 'English fingerprints',
+            known_hosts: 'English known hosts',
+          },
         },
       },
     })
@@ -54,12 +64,22 @@ describe('get-data', () => {
             },
           },
           variables: {
+            copilot: {
+              prodname_copilot: 'Translated Copilot',
+            },
+            product: {
+              company_short: 'Translated GitHub',
+            },
             stuff: {
               foo: 'フー',
             },
           },
           reusables: {
             coolness: 'これがマークダウンです',
+            ssh: {
+              fingerprints: 'Translated fingerprints',
+              known_hosts: 'Translated known hosts',
+            },
           },
         },
       },
@@ -102,6 +122,17 @@ describe('get-data', () => {
     }
   })
 
+  test('getDataByLanguage always reads selected variable data from English', () => {
+    {
+      const result = getDataByLanguage('variables.product.company_short', 'ja')
+      expect(result).toBe('GitHub')
+    }
+    {
+      const result = getDataByLanguage('variables.copilot.prodname_copilot', 'ja')
+      expect(result).toBe('GitHub Copilot')
+    }
+  })
+
   test('getDataByLanguage variables failures', () => {
     {
       const result = getDataByLanguage('variables.stuff.key_non_existent', 'en')
@@ -117,6 +148,11 @@ describe('get-data', () => {
     }
   })
 
+  test('getDataByLanguage uses configured English root for always-English missing keys', () => {
+    const result = getDataByLanguage('variables.product.prodname_dotcom', 'ja')
+    expect(result).toBeUndefined()
+  })
+
   test('getDataByLanguage reusables English', () => {
     {
       const result = getDataByLanguage('reusables.coolness', 'en')
@@ -139,6 +175,17 @@ describe('get-data', () => {
     }
   })
 
+  test('getDataByLanguage always reads selected SSH reusables from English', () => {
+    {
+      const result = getDataByLanguage('reusables.ssh.fingerprints', 'ja')
+      expect(result).toBe('English fingerprints')
+    }
+    {
+      const result = getDataByLanguage('reusables.ssh.known_hosts', 'ja')
+      expect(result).toBe('English known hosts')
+    }
+  })
+
   test('getDataByLanguage failures', () => {
     {
       const result = getDataByLanguage('reusables.neverheardof', 'en')
@@ -175,6 +222,16 @@ describe('get-data', () => {
       expect(result['coolness.md']).toBe('This is *Markdown*')
     }
   })
+
+  test('getDeepDataByLanguage uses configured English root for always-English files', () => {
+    const variables = getDeepDataByLanguage('variables', 'en')
+    expect((variables.product as Record<string, string>).prodname_dotcom).toBeUndefined()
+    expect((variables.copilot as Record<string, string>).prodname_copilot).toBe('GitHub Copilot')
+
+    const sshReusables = getDeepDataByLanguage('reusables.ssh', 'en')
+    expect(sshReusables['fingerprints.md']).toBe('English fingerprints')
+    expect(sshReusables['known_hosts.md']).toBe('English known hosts')
+  })
 })
 
 const VALID_ENGLISH_MARKDOWN = `