Repository navigation
chore(content-server): remove dead grunt build steps, tailwind and app/images - #21659
Open
fxa-agent[bot] wants to merge 1 commit into
Open
fxa-agent[bot] wants to merge 1 commit into
fxa-agent[bot] wants to merge 1 commit into
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Remove page-template rendering and processing packages/fxa-content-server/server/lib/configuration.js, packages/fxa-content-server/server/config/*, packages/fxa-content-server/server/bin/fxa-content-server.js, packages/fxa-content-server/grunttasks/yeoman.js, packages/fxa-content-server/grunttasks/{clean,htmlmin,copy,cdnify,usemin}.js |
Page-template settings and Express view setup were removed. Grunt page-template processing and copying tasks were removed or updated. |
Remove Tailwind and CSS build packages/fxa-content-server/app/styles/tailwind*, packages/fxa-content-server/tailwind.config.js, packages/fxa-content-server/postcss.config.js, packages/fxa-content-server/package.json, packages/fxa-content-server/pm2.config.js |
Tailwind and PostCSS configuration, CSS build commands, and the PM2 content-css app were removed. start-production no longer runs the CSS build. |
Remove distribution processing tasks packages/fxa-content-server/grunttasks/{concat,cssmin,replace,rev,sriify}.js, packages/fxa-content-server/grunttasks/copy.js, packages/fxa-content-server/package.json, packages/fxa-content-server/.prettierignore |
Distribution processing tasks and their dependencies were removed. Static asset copy patterns and Prettier ignore paths were updated. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Other
Possibly related PRs
- mozilla/fxa#21650: Removes the remaining content-server localization and generated-page build pipeline.
- mozilla/fxa#21648: Replaces the content-server-rendered HTML 404 page with the React Settings page.
Suggested reviewers: dschom, lzoog, vpomerleau
Merge Risk: ⚪ Minimal · up to 00a99
The remaining page and asset paths are supported by the reduced build. No concrete merge-blocking issue was established.
Architecture Summary
Architecture risk: 🔵 Low · up to 00a99
The change affects 1 system.
Changed systems: packages/fxa-content-server
Architecture concerns
No architecture-level concerns identified.
Review details
-
Systems and components
- observed — packages/fxa-content-server (library) was modified; 23 changed files map to changed impact.
Before / after behavior
- observed — Modified behavior in packages/fxa-content-server/.prettierignore: The ignore list includes locale, localization, distribution, and test certificate paths. The previous
app/images/*andserver/templates/pages/dist/*exclusions were removed. - observed — Modified behavior in packages/fxa-content-server/app/styles/tailwind.css: The entire stylesheet was removed, including its Tailwind and React style imports, CTA import, and
bodyutility rules. - observed — Modified behavior in packages/fxa-content-server/app/styles/tailwind/README.md: The README documenting Tailwind usage, stylesheet and template considerations, and component-class guidance was removed.
- observed — Modified behavior in packages/fxa-content-server/app/styles/tailwind/ctas.css: The stylesheet was deleted, removing the
.cta-primary .spinnerbackground and utility rules and the white text color rule for focuseda.cta-primarylinks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Title check The title uses the required type, scope, and imperative lowercase subject, and it describes the changes. It is 77 characters, which exceeds the 72-character limit. Shorten the title to 72 characters or fewer while preserving the main change, for example: "chore(content-server): remove unused build steps and Tailwind". -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Sensitive Data Protection And Privacy The pull request introduces no sensitive-data logging, metrics, Sentry events, error messages, credentials, third-party PII transfer, data retention, or isolation changes. The reviewed diff mainly del… Security, Audit, And Regulatory Control Integrity No explicit security-control failure is introduced. The diff removes obsolete build and view-template tooling, Tailwind assets, and static images. In server/bin/fxa-content-server.js, only the Handl…Performance And Algorithmic Complexity Review The PR does not introduce a performance or complexity defect covered by this check. The diff mainly deletes unused Grunt, Tailwind, image, and template-build code. The only executable additions are a … Database Migration Safety The pull request changes no files under packages/db-migrations/databases/*/patches/and no migration metadata. It introduces no schema change, so the forward/reverse patch and target-patch requireme…Full details: Sensitive Data Protection And Privacy
-
Explanation
The pull request introduces no sensitive-data logging, metrics, Sentry events, error messages, credentials, third-party PII transfer, data retention, or isolation changes. The reviewed diff mainly deletes build tasks, static assets, Tailwind files, dependencies, and the Handlebars view engine. The only added executable settings are build/copy commands. Existing metrics and request logging code is unchanged.
Full details: Security, Audit, And Regulatory Control Integrity
-
Explanation
No explicit security-control failure is introduced. The diff removes obsolete build and view-template tooling, Tailwind assets, and static images. In
server/bin/fxa-content-server.js, only the Handlebars engine and template-directory setup are removed; Helmet, CSP, cookie parsing, body parsing, route logging, and route registration remain. No authentication, session, token, OAuth validation, rate limiting, webhook verification, payment control, or account-event implementation changes appear in the PR. The removedsriifytask only added SRI attributes to obsolete server-rendered HTML and is not Stripe, PayPal, Apple, or Google webhook signature verification.
Full details: Performance And Algorithmic Complexity Review
-
Explanation
The PR does not introduce a performance or complexity defect covered by this check. The diff mainly deletes unused Grunt, Tailwind, image, and template-build code. The only executable additions are a narrower copy glob and simplified build scripts. No new request handlers, loops, datastore calls, listeners, buffers, or render work were added. The server’s existing body parsing and route setup remain unchanged; removing the PM2 CSS worker reduces background work.
Full details: Database Migration Safety
-
Explanation
The pull request changes no files under
packages/db-migrations/databases/*/patches/and no migration metadata. It introduces no schema change, so the forward/reverse patch and target-patch requirements do not apply.
-
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Note
To fit the comment limit, these sections were omitted: recent review.
LZoog
reviewed
Oct 30, 2026
| @@ -33,16 +33,5 @@ module.exports = function (grunt) { | |||
| ], | |||
| options: minOptions, | |||
Contributor
There was a problem hiding this comment.
What do we need htmlmin for now if there's no used HTML files?
Contributor
Author
There was a problem hiding this comment.
One HTML file is left: app/crash.html. htmlmin appFiles (src *.html) is the only step that puts it into dist, because copy:dist now copies only *.{ico,png,txt} and .htaccess (grunttasks/copy.js). If crash.html is not needed, both can go in a follow-up.
This comment is from an AI agent.
Contributor
There was a problem hiding this comment.
Filed a follow up to address this https://mozilla-hub.atlassian.net/browse/FXA-14896
| @@ -22,7 +22,6 @@ module.exports = function (grunt) { | |||
| '<%= yeoman.es5 %>/*', | |||
| '<%= yeoman.dist %>/*', | |||
Contributor
There was a problem hiding this comment.
Do we still need yeoman / yeoman generators now in content-server? Or is there a more modern option (would just using vite give us what we want)? Probably a follow up.
Contributor
Author
There was a problem hiding this comment.
No yeoman package or generator is in the content server dependencies. grunttasks/yeoman.js is only a grunt config object with path names (app, dist, es5, tmp) that other grunt tasks read. A move from grunt to vite or plain scripts is wider than this ticket, so it fits a follow-up.
This comment is from an AI agent.
Contributor
There was a problem hiding this comment.
Filed a follow up to address this https://mozilla-hub.atlassian.net/browse/FXA-14896
…p/images Because: * The generated 404 pages are gone, so most content-server grunt steps, the view engine, tailwind and app/images have no input or user. This commit: * Deletes the dead grunt tasks and copy, htmlmin and template config. * Removes the handlebars view engine and the page_template config. * Removes tailwind, app/styles, app/images and the build-css script. * Removes the unused dependencies and updates yarn.lock. Closes FXA-14887 Co-authored-by: Lauren Zugai <lauren@zugai.com>
LZoog
approved these changes
Oct 30, 2026
LZoog
left a comment
LZoog
left a comment
Contributor
There was a problem hiding this comment.
- I verified the images in
app/imageshaven't been used in emails since June 2020 - Started the stack locally and poked around Settings without problems
- I've filed yet another follow up for more deletions: https://mozilla-hub.atlassian.net/browse/FXA-14896
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
app/images, but the package still installs 12 dependencies for them.This pull request
cdnify,concat,cssmin,replace,rev,sriifyandusemingrunt tasks.error_pages,mozillaonlineandstylescopy targets and the images glob incopy:dist.htmlmintemplatestarget. TheappFilestarget stays forcrash.html.page_templateentries inclean.jsandyeoman.js.consolidateand the handlebars view engine fromserver/bin/fxa-content-server.js. Nores.rendercall exists.page_template_rootandpage_template_subdirectoryconfig, also fromproduction.jsonandlocal.json-dist.build-cssscript and its calls inbuildandstart-production, thecontent-csspm2 process,postcss.config.js,tailwind.config.jsandapp/styles.app/images(154 files).yarn.lock:consolidate,handlebars,grunt-cdn,grunt-contrib-concat,grunt-contrib-cssmin,grunt-file-rev,grunt-sri,grunt-text-replace,grunt-usemin,postcss-assets,postcss-importandtailwindcss.build-fasttoexit 0, as infxa-auth-server,fxa-settingsandfxa-react. This wayprebuildstill runs underyarn start mza.Risk
Risk: High (the worst of the three below)
Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-14887
Checklist
Put an
xin the boxes that applyHow to review (Optional)
server/bin/fxa-content-server.js,grunttasks/copy.jsand thescriptskey inpackage.json.package.json, the grunt tasks, then the server file.app/images. See "Before merge".Screenshots (Optional)
No user interface change.
Other information (Optional)
Before merge: A reviewer checks the content server access logs for /images/* requests before merge. This PR did not check them.
Testing. I ran these checks in the sandbox:
grunt buildpasses.disthascrash.htmlandsettings/. It has noimages/and nostyles/.server/libmocha specs pass. They include404.test.jsand500.test.js.tscpass on the changed files./metricswith 500.CI covers the Docker build and the full suites.
Notes for reviewers:
scriptskey ofpackages/fxa-content-server/package.json. It removesbuild-cssand changesbuild,build-fastandstart-production, as the ticket asks._dev/docker/docker-compose.yml:45still setsPAGE_TEMPLATE_SUBDIRECTORY=dist. It is outside the scope of this PR. Convict ignores an env var that has no schema key. A follow-up can remove it.Merge Confidence: 3/5
This change removes the content-server’s unused page-template and CSS build pipeline, including Tailwind, the Handlebars view engine, related Grunt tasks, configuration, and dependencies. It also changes
build-fastto exit successfully without building CSS and removes thecontent-cssPM2 app.The author reports that
grunt build, theserver/libMocha specs, and linting and type checks on changed files pass. Local checks reportedly confirm JSON responses for an unknown path and a bad JSON POST. The author did not verify production Docker builds or check access logs for requests to/images/*, so confirm those before merge.