chore(email-renderer): Flatten types for renderer and fix some tests #19901

nshirley · 2026-01-16T20:35:40Z

Because

The types on fxa-email-renderer render functions make them difficult to use in auth-server
And there was some cleanup to do from the first integration with email-sender to auth-server

This pull request

Flattens the types on fxa-email-renderer functions
Updates snapshot tests (even though they're still skipped)
Updates the fxa-mailer to use the flattened type
Fixes first libs email sender test to use deepEquals assert on params passed to mailer
Adds a formatGeoData to convert app request geo location data to format expected by library

Issue that this pull request solves

Closes: FXA-12885

Checklist

Put an x in the boxes that apply

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.
I have added necessary documentation (if appropriate).
I have verified that my changes render correctly in RTL (if appropriate).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Any other information that is important to this pull request.

nshirley · 2026-01-16T20:38:25Z

libs/accounts/email-renderer/src/renderer/fxa-email-renderer.spec.ts

These are all the snapshot tests updated after changing the renderer types

Of note too, I did away with the overly complex "renderAndSnapshotTest" function... I got too fancy and it made things too complex

nshirley · 2026-01-16T20:41:50Z

libs/accounts/email-renderer/src/renderer/fxa-email-renderer.ts

 import * as VerifySecondaryCode from '../templates/verifySecondaryCode';
 import * as VerifyShortCode from '../templates/verifyShortCode';

+export type WithFxaLayouts<T> = T & FxaLayouts.TemplateData;


@dschom - Since all of these need to be paired with the FxaLayouts.TemplateData this felt okay. The other idea I had was to declare/export all of the input types here, then they can be used in the fxa-mailer.ts explicitly, so they stay in sync, something like this:

export type RenderRecoveryInput = WithFxaLayouts<Recovery.TemplateData>; // on and on

Feels a bit like overkill, but also means they can be used in the fxa-mailer and we don't have to then do the same WithFxaLayouts<Recovery.TemplateData>; inside the fxa-mailer on the send functions

IMO, a bunch of redeclaration seems unnecessary to me and just adds maintenance work. And using WithFxaLayouts<Recovery.TemplateData> in fxa-mailer feels totally okay to.

nshirley · 2026-01-16T20:43:37Z

packages/fxa-auth-server/lib/senders/fxa-mailer.ts

-      OmitCommonLinks<renderer.TemplateData> &
-      OmitCommonLinks<renderer.passwordForgotOtp.TemplateData>
+      OmitCommonLinks<
+        renderer.WithFxaLayouts<renderer.passwordForgotOtp.TemplateData>


Yeah, this is where we could be simplier if we exported the input types

OmitCommonLinks<renderer.PasswordForgotOtpInput>

I think this is fine. I sort of wonder if just changing the import declaration would clean this up e.g. drop * as renderer thing and just do import { WithFxaLayouts, passwordFortgotOtp } from '@fxa/accounts/email-renderer';

packages/fxa-auth-server/test/local/routes/password.js

nshirley · 2026-01-16T20:48:45Z

packages/fxa-shared/lib/geo-data.ts

+ * @param geoData Object containing city, state, and country
+ * @returns
+ */
+export function formatGeoData({


This kind of feels like a helper for the sake of a helper... We could just align the type inside email-renderer with the same names... but I fear that would be more work than it's worth

I think this is fine approach.

dschom · 2026-01-16T21:36:23Z

packages/fxa-auth-server/test/local/routes/password.js

-    const uid = uuid.v4({}, Buffer.alloc(16)).toString('hex');
-    const passwordForgotTokenId = crypto.randomBytes(16).toString('hex');
+    const uid = '6bfe0db9d4b44ccaa7c175a889024b86';
+    const passwordForgotTokenId =


I will sometimes make these values more 'test' like. ie Fill the string 1234678, or all zeros... When using a hard coded random values like this, I've seen code analysis will flag and generate a warning about committed token.

dschom · 2026-01-16T21:43:55Z

libs/accounts/email-renderer/src/renderer/fxa-email-renderer.ts

 import * as VerifySecondaryCode from '../templates/verifySecondaryCode';
 import * as VerifyShortCode from '../templates/verifyShortCode';

+export type WithFxaLayouts<T> = T & FxaLayouts.TemplateData;


IMO, a bunch of redeclaration seems unnecessary to me and just adds maintenance work. And using WithFxaLayouts<Recovery.TemplateData> in fxa-mailer feels totally okay to.

dschom · 2026-01-16T21:44:17Z

libs/accounts/email-renderer/src/index.spec.ts

-        sync: false,
-      }
-    );
+    const email = await r.renderAdminResetAccounts({


dschom · 2026-01-16T21:45:05Z

packages/fxa-auth-server/lib/routes/password.ts

-            country: geoData.location?.country,
-            stateCode: geoData.location?.state,
-          },
+          location: formatGeoData(geoData.location),


dschom · 2026-01-16T21:47:52Z

packages/fxa-auth-server/lib/senders/fxa-mailer.ts

-      OmitCommonLinks<renderer.TemplateData> &
-      OmitCommonLinks<renderer.passwordForgotOtp.TemplateData>
+      OmitCommonLinks<
+        renderer.WithFxaLayouts<renderer.passwordForgotOtp.TemplateData>


I think this is fine. I sort of wonder if just changing the import declaration would clean this up e.g. drop * as renderer thing and just do import { WithFxaLayouts, passwordFortgotOtp } from '@fxa/accounts/email-renderer';

- The types on fxa-email-renderer render functions make them difficult to use in auth-server - And there was some cleanup to do from the first integration with email-sender to auth-server - Flattens the types on fxa-email-renderer functions - Updates snapshot tests (even though they're still skipped) - Updates the fxa-mailer to use the flattened type - Fixes first libs email sender test to use deepEquals assert on params passed to mailer - Adds a formatGeoData to convert app request geo location data to format expected by library Closes: FXA-12885

nshirley requested a review from a team as a code owner January 16, 2026 20:35

nshirley force-pushed the FXA-12885 branch from 7f48f06 to 8b82416 Compare January 16, 2026 20:36

nshirley commented Jan 16, 2026

View reviewed changes

packages/fxa-auth-server/test/local/routes/password.js Show resolved Hide resolved

nshirley commented Jan 16, 2026

View reviewed changes

nshirley force-pushed the FXA-12885 branch 2 times, most recently from 3d970b6 to e87fdec Compare January 16, 2026 21:34

dschom approved these changes Jan 17, 2026

View reviewed changes

nshirley force-pushed the FXA-12885 branch from e87fdec to 7ec8154 Compare January 18, 2026 23:35

chore(email-renderer): Flatten types for renderer and fix some tests #19901

Are you sure you want to change the base?

chore(email-renderer): Flatten types for renderer and fix some tests #19901

Uh oh!

Conversation

nshirley commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Because

This pull request

Issue that this pull request solves

Checklist

Screenshots (Optional)

Other information (Optional)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nshirley Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nshirley commented Jan 16, 2026 •

edited

Loading

nshirley Jan 16, 2026 •

edited

Loading