Use sync Localization methods when required

README.md

@@ -22,7 +22,7 @@ When migrating legacy messages, multiple things change:
 2. The file's location within `mozilla-central` changes.
 3. Message keys change, and often gain an identifying prefix in addition to being kebab-cased.
 4. The syntax for referring to variables in messages changes.
-5. The JavaScript API for formatting messages changes, and becomes async.
+5. The JavaScript API for formatting messages changes.
 
 To help with the first three, you need to add some metadata comments to each `.properties` file that you're migrating:
 
@@ -54,7 +54,6 @@ All of those files are then modified in-place.
 
 Because so many things change, it's unlikely that the script will catch everything.
 Where possible, a comment `/* L10N-FIXME */` is injected immediately after points in the source that require human attention.
-Additionally, the effects of making functions that contain formatting calls `async` will need to be reviewed.
 
 ## TODO
 

lib/external-refs.js

@@ -60,6 +60,7 @@ export async function findExternalRefs(root, ast) {
    *   path: string,
    *   msgKeys: string[],
    *   migrated: Record<string, string>,
+   *   requiresSync: boolean
    *   ftlPath: string | null,
    *   ftlRoot: string | null,
    *   ftlPrefix: string,
@@ -85,6 +86,7 @@ export async function findExternalRefs(root, ast) {
           path: fp,
           msgKeys,
           migrated: {},
+          requiresSync: false,
           ftlPath,
           ftlRoot,
           ftlPrefix,

lib/transform-js.js

@@ -41,7 +41,6 @@ export async function transformJs(jsPath, { dryRun, root } = {}) {
   }
 
   const fixmeNodes = new Set()
-  let didMakeAsync = false
 
   const bundleInitMethods = new Set([
     'createBundle', // Services.strings.createBundle()
@@ -102,17 +101,6 @@ export async function transformJs(jsPath, { dryRun, root } = {}) {
     }
   })
 
-  for (const [ftlPath, nodePaths] of bundlePaths.entries()) {
-    if (nodePaths.length === 1) continue
-    let name = 'gL10n'
-    if (bundlePaths.size > 1) {
-      const fn = basename(ftlPath, '.ftl').replace(/\W/g, '')
-      name += fn[0].toUpperCase() + fn.substring(1)
-    }
-    addLocalizationGetter(nodePaths[0], name, ftlPath)
-    for (const np of nodePaths) np.replace(b.identifier(name))
-  }
-
   // name -> arguments.length
   const fmtMethods = new Map([
     ['GetStringFromName', 1], // Services.strings.createBundle()
@@ -121,6 +109,7 @@ export async function transformJs(jsPath, { dryRun, root } = {}) {
     ['getFormattedString', 2] // <stringbundle>
   ])
 
+  let requiresSync = false
   const msgKeyLiterals = new Map()
   const fmtArgObjects = new Set()
   visit(ast, {
@@ -162,13 +151,27 @@ export async function transformJs(jsPath, { dryRun, root } = {}) {
           path.get('arguments', 1)
         )
         const res = setLocalizationCall(path, key, ftlMsg, fmtArgs)
-        if (res === 'fixme') fixmeNodes.add(key)
-        else if (res === 'async') didMakeAsync = true
+        if (res.fixme) fixmeNodes.add(key)
+        if (!res.isAsync) requiresSync = true
         if (args[1] && !fmtArgs) fixmeNodes.add(args[1])
       }
     }
   })
 
+  for (const [ftlPath, nodePaths] of bundlePaths.entries()) {
+    if (nodePaths.length === 1) {
+      if (requiresSync) nodePaths[0].node.arguments[1] = b.literal(true)
+    } else {
+      let name = 'gL10n'
+      if (bundlePaths.size > 1) {
+        const fn = basename(ftlPath, '.ftl').replace(/\W/g, '')
+        name += fn[0].toUpperCase() + fn.substring(1)
+      }
+      addLocalizationGetter(nodePaths[0], name, ftlPath, requiresSync)
+      for (const np of nodePaths) np.replace(b.identifier(name))
+    }
+  }
+
   const fixmeLines = new Set()
   if (fixmeNodes.size > 0) {
     for (const node of fixmeNodes) {
@@ -222,10 +225,6 @@ export async function transformJs(jsPath, { dryRun, root } = {}) {
   // Update JS file
   if (!dryRun) await writeFile(jsPath, print(ast).code)
   console.warn(`Patched ${relative(root, jsPath)}`)
-  if (didMakeAsync)
-    console.warn(
-      `  !!! Some functions were made async. Check their call sites.`
-    )
   if (fixmeLines.size > 0)
     console.warn(
       `  !!! Fix L10N-FIXME issues manually near lines:`,
@@ -328,21 +327,38 @@ function fixFormatterArgs(done, path) {
 }
 
 function setLocalizationCall(path, key, ftlMsg, fmtArgs) {
-  const fmtCall = path.node
-  let res = null
+  let fmtCall = path.node
+  let fixme = false
+
+  let scopeFn = path.parent
+  while (scopeFn && !n.Function.check(scopeFn.node)) scopeFn = scopeFn.parent
+  const isAsync = !!scopeFn?.node.async
 
   if (!ftlMsg || !ftlMsg.attr) {
-    // await bundle.formatValue(key, fmtArgs)
-    fmtCall.callee.property.name = 'formatValue'
-    if (fmtArgs) fmtCall.arguments[1] = fmtArgs
-    path.replace(b.awaitExpression(fmtCall))
+    if (isAsync) {
+      // await bundle.formatValue(key, fmtArgs)
+      fmtCall.callee.property.name = 'formatValue'
+      if (fmtArgs) fmtCall.arguments[1] = fmtArgs
+      path.replace(b.awaitExpression(fmtCall))
+    } else {
+      // bundle.formatValueSync(key, fmtArgs)
+      fmtCall.callee.property.name = 'formatValueSync'
+      if (fmtArgs) fmtCall.arguments[1] = fmtArgs
+    }
   } else {
-    // bundle.formatMessages([{ id: key, args: fmtArgs }]).attributes.attr
-    fmtCall.callee.property.name = 'formatMessages'
     const fmtProps = [b.objectProperty(b.identifier('id'), key)]
     if (fmtArgs) fmtProps.push(b.objectProperty(b.identifier('args'), fmtArgs))
     fmtCall.arguments = [b.arrayExpression([b.objectExpression(fmtProps)])]
 
+    if (isAsync) {
+      // await bundle.formatMessages([{ id: key, args: fmtArgs }])
+      fmtCall.callee.property.name = 'formatMessages'
+      fmtCall = b.awaitExpression(fmtCall)
+    } else {
+      // bundle.formatMessagesSync([{ id: key, args: fmtArgs }])
+      fmtCall.callee.property.name = 'formatMessagesSync'
+    }
+
     let attr
     if (n.Literal.check(key)) {
       const fa = ftlMsg.attr
@@ -350,40 +366,32 @@ function setLocalizationCall(path, key, ftlMsg, fmtArgs) {
       key.value = ftlMsg.key
     } else {
       attr = b.identifier('FIXME')
-      res = 'fixme'
+      fixme = true
     }
 
     path.replace(
-      b.awaitExpression(
-        b.memberExpression(
-          b.memberExpression(fmtCall, b.identifier('arguments')),
-          attr
-        )
+      b.memberExpression(
+        b.memberExpression(fmtCall, b.identifier('attributes')),
+        attr
       )
     )
   }
 
-  let scopeFn = path.parent
-  while (scopeFn && !n.Function.check(scopeFn.node)) scopeFn = scopeFn.parent
-  if (scopeFn && !scopeFn.node.async) {
-    scopeFn.node.async = true
-    if (!res) res = 'async'
-  }
-
-  return res
+  return { fixme, isAsync }
 }
 
-function addLocalizationGetter(path, name, ftlPath) {
+function addLocalizationGetter(path, name, ftlPath, requiresSync) {
   const utilsPath = getUtilsStatementPath(path)
   const { body } = utilsPath.parent.node
   const insertPos = body.indexOf(utilsPath.node) + 1
 
   const jsName = JSON.stringify(name)
   const jsFtl = JSON.stringify(ftlPath)
+  const ctorArgs = requiresSync ? `${jsFtl}, true` : jsFtl
   const getter = jsParse(
     `
 XPCOMUtils.defineLazyGetter(this, ${jsName}, () =>
-  new Localization([${jsFtl}])
+  new Localization([${ctorArgs}])
 );
 `
   ).program.body[0]