chore(auth): Apply eslint to all auth-server test files#20792
Conversation
Because: * An unanchored "fxa-*" ignorePattern matched by basename at any depth, silently excluding real source and test files from ESLint. This commit: * Anchors the pattern to "/fxa-*" so it only ignores top-level vendored dirs. * Fixes the latent lint errors the un-ignore surfaces. * Documents the spec-only async-crypto-random relaxation. Fixes #FXA-13283
nshirley
changed the title
| * argument shapes of each `fxaMailer.send*` invocation. Their being unused is by design, | ||
| * so `no-unused-vars` is disabled for the whole file. | ||
| */ | ||
| /* eslint-disable @typescript-eslint/no-unused-vars */ |
There was a problem hiding this comment.
Honestly, we could probably remove this file altogether. It was useful when rolling out the fxa-mailer and libs/email/ integration since not all call sites are TS. But, it doesn't serve any other purpose now
There was a problem hiding this comment.
Pull request overview
This PR tightens packages/fxa-auth-server’s ESLint ignore pattern so it only ignores top-level vendored fxa-* directories, ensuring real source/tests (e.g., lib/senders/fxa-mailer*.ts) are linted, and fixes the lint issues that were previously being silently skipped.
Changes:
- Anchor the ignore pattern from
fxa-*to/fxa-*and document why, plus document the spec-onlyasync-crypto-randomrelaxation. - Adjust a mailer spec to avoid an unused-destructuring pattern while keeping the “no event recorded without uid” behavior.
- Minor lint-driven cleanup (e.g.,
let→const) and add an explicit file-levelno-unused-varsdisable for the type-check-only sanity file.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/fxa-auth-server/lib/senders/fxa-mailer.spec.ts | Updates a test setup to avoid linting issues while still exercising the “no uid → no event” path. |
| packages/fxa-auth-server/lib/senders/fxa-mailer-sanity-check.ts | Clarifies intent of the type-check-only “sanity” driver and disables no-unused-vars for the file. |
| packages/fxa-auth-server/lib/senders/fxa-mailer-format.ts | Replaces a non-reassigned let with const to satisfy linting. |
| packages/fxa-auth-server/.eslintrc.json | Anchors the ignore pattern and adds explanatory comments for the ignore/override behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * This is jsut a strong typed driver that allows us to validate the params we pass into FxaMailer | ||
| * functions are type safe. Since there's still a lot of code in auth-server that isn't type safe | ||
| * this gives us some level of sanity that we are invoking the mailer methods correctly. |
| }; | ||
| }) { | ||
| let metricsContext = await request.app.metricsContext; | ||
| const metricsContext = await request.app.metricsContext; |
There was a problem hiding this comment.
Caught by enabling the linting on correct file paths. Prior unanchored fxa-* would catch any file named as such, ignoring this and the fxa-mailer files
| "ignorePatterns": [ | ||
| "dist", | ||
| "fxa-*", | ||
| // Anchored to the package root so it only ignores top-level vendored fxa-* |
There was a problem hiding this comment.
Probably don't need the comments, but they did feel relevant. I'm indifferent if they stay or go!
Because:
This commit:
Closes: #FXA-13283
Checklist
Put an
xin the boxes that applyHow to review (Optional)
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.