This is awesome, thanks Stas!

I think this is now ready for a review. I tested this locally after merging master into the branch (I didn't include the merge in the PR), and things looked good. I don't know the whole app, however, so I'll be happy to do more manual testing if you'd like me to.

@phirework Can you help me find a reviewer for this, please?

@stasm Apologies for the delay, I'll take a more detailed look today or tomorrow. We've been busy with our migration to Kubernetes and I didn't want to merge in a big change before also changing server environments.

Of course, no worries! I just wanted to make sure this is on your radar.

I've also just checked locally, and it looks like there haven't been any new <Localized> added since I created this PR, so it should be good to merge. More than bitrot, I was worried that there would be new code added on master which would use the old Localized API, and which would go unnoticed during the merge.

Sorry for the late update! So I've finally had a chance to look through everything in more detail. Thanks again for doing this work.

So the big piece blocking this is that not all of the places we make use of the <Localized> object directly references the element, instead it's being passed around in props. You can see missing variable warnings on /speak, /languages, /datasets, etc. So for example, in contribute.tsx, these changes need to be made for it to be passed down to speak.tsx and listen.tsx:

@@ -91,7 +91,7 @@ interface Props extends WithLocalizationProps, PropsFromState {
   errorContent?: any;
   reportModalProps: Omit<ReportModalProps, 'onSubmitted'>;
   instruction: (props: {
-    $actionType: string;
+    vars: { actionType: string; }
     children: any;
   }) => React.ReactNode;
   isFirstSubmit?: boolean;

@@ -423,7 +423,7 @@ class ContributionPage extends React.Component<Props, State> {

               <div className="cards-and-instruction">
                 {instruction({
-                  $actionType: getString('action-click'),
+                  vars: { actionType: getString('action-click') },
                   children: <div className="instruction hidden-sm-down" />,
                 }) || <div className="instruction hidden-sm-down" />}

@@ -500,7 +500,7 @@ class ContributionPage extends React.Component<Props, State> {
             </div>

             {instruction({
-              $actionType: getString('action-tap'),
+              vars: { actionType: getString('action-tap') },
               children: <div className="instruction hidden-md-up" />,
             }) || <div className="instruction hidden-md-up" />}

Off the top of my head, the most fail-safe way to look for all of these would be to start with web/locales/en/messages.ftl and check all the translation strings that make use of elements or variables. That sounds very tedious and boring and I don't know if you would consider that in-scope for the work you're doing. Is this pull request a blocker to you all releasing a new version on your side?

It should be fairly easy for me to collect the names of variables and elements used in all messages, and then grep for them in the code. I didn't realize they were passed like this in objects, thanks for pointing it out.

I'm not blocked on this PR, thanks for asking. I already released @fluent/react 0.12 some time ago. I'd like to help to get this PR landed so that it doesn't bitrot completely, making it hard to upgrade to new versions of @fluent/react in the future.

I updated the PR to fix the cases where the props to Localized are passed around as objects. I think I got them all. I first queried the localization files in web/locales/en to look for all variables and markup used in the translations. I then grepped the source in web/src and reviewed each occurence.

One tricky piece of code was this one:

https://github.com/mozilla/voice-web/blob/fd9d7c7c5bf45b438551c71c6ecb6ddeb8cf4a69/web/src/components/pages/datasets/dataset-info.tsx#L332-L338

It kind of demonstrates why the old API wasn't ergnomic. In the new API, the above becomes simply:

<Localized
    id="dataset-description-hours"
    vars={globalStats}
    elems={{b: <b />}}
>

I also found two messages which may need some work:

• dataset-description-hours references the languagesLink element which doesn't seem to be passed into the corresponding Localized.

• data-get-started doesn't seem to be used anywhere. It references the speechBlogLink element which also seems to be missing from the codebase.

@phirework Would you mind taking another look at this?

This is awesome, thank you! I will take care of cleaning those two instances up separately, good catch. Super appreciate all your work here.

Thanks, @phirework!