chore: slim Rocket.Chat Docker image (~260 MB, -14.7%)#40128
Conversation
…uild The livechat widget's webpack build emits full source-map files (~12 MB in a fresh build, far more with accumulated rebuilds). They are copied into apps/meteor/public/livechat/ by the rocketchat-livechat plugin and end up in the Meteor bundle and Docker image without adding runtime value. - plugin/build.sh: delete .map files after copying dist into public. Maps remain in packages/livechat/dist for local debugging or upload to error tracking. - packages/livechat: prefix build with yarn clean so dist no longer accumulates stale hashed chunks across rebuilds (noticed dist growing to ~500 MB locally vs. ~18 MB fresh).
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughAdds post-build cleanup of sourcemap files and minor build/script adjustments across Livechat, Docker build stages, and font assets; also restricts removed maps to locations safe for runtime. Includes a WOFF2-only font reference change. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.7.0)apps/meteor/app/theme/client/rocketchat.font.cssConfigurationError: Could not find "stylelint-order". Do you need to install the package or use the "configBasedir" option? Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40128 +/- ##
=========================================
Coverage 70.20% 70.21%
=========================================
Files 3278 3280 +2
Lines 116605 116852 +247
Branches 20723 20711 -12
=========================================
+ Hits 81859 82042 +183
- Misses 31444 31526 +82
+ Partials 3302 3284 -18
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Two small gaps vs. standard Docker image hygiene: - Dockerfile.alpine: add `npm cache clean --force` after the prod install in bundle/programs/server so the cache doesn't ship in the final image layer. - Dockerfile.debian: pass `--omit=dev` to both npm installs to match the alpine variant, avoiding devDependencies of meteor-dev-bundle (split2, multipipe, chalk) in the production image.
- apps/meteor/public/fonts/KaTeX_*.{ttf,woff,woff2} (60 files, ~1 MB):
orphan. The main app does not reference them in any CSS, SCSS, or
JS/TS source — only the livechat widget uses KaTeX, and it bundles
its own hashed copies via webpack's file-loader. These were left
over from when the main app used KaTeX directly.
- apps/meteor/public/fonts/rocketchat.{eot,svg,ttf,woff} (4 files,
~0.7 MB) and the corresponding @font-face fallbacks in
app/theme/client/rocketchat.font.css: with `browserslist: ["last 2
versions", "Firefox ESR"]`, every target supports WOFF2. EOT (IE),
SVG fonts (iOS <4.2), TTF and WOFF are unused fallbacks.
After `npm install --omit=dev` in the builder stage, delete every `*.map` file inside /app/bundle. Node does not consume sourcemaps at runtime; the bytes come from `.map` files shipped inside third-party packages (openpgp, date-fns, meteor/*, etc.) and from server/app.js.map. Measured on a flattened rocketchat/rocket.chat:develop image: - before: 1.78 GB - after: 1.50 GB (-280 MB / -15.7%) - 18,285 .map files removed `-type f` guards against directories named `*.map` (e.g. the `array.prototype.map` npm package).
7d74923 to
725bddb
Compare
|
/jira ARCH-2083 |
The previous commit assumed the 60 KaTeX_*.{woff2,woff,ttf} files in
apps/meteor/public/fonts/ were orphan because no code in the repo
references them by name. That was wrong.
gazzodown imports katex/dist/katex.css, which contains @font-face rules
with relative URLs like url(fonts/KaTeX_Main-Regular.woff2). The Meteor
bundler copies katex.css into the dynamic chunk literally, without
processing url() or copying the katex/dist/fonts/ directory alongside
it. When the stylesheet is injected as a <style> element at runtime,
the browser resolves url(fonts/...) against the document root (/), so
the font requests land at /fonts/KaTeX_*.woff2 — served by Meteor from
apps/meteor/public/fonts/.
Removing the files means those requests hit the Meteor SPA catch-all,
which returns index.html with HTTP 200. The browser accepts the 200
but fails to parse the HTML as a font, all three @font-face fallbacks
fail (woff2 -> woff -> ttf, all the same HTML response), and rendering
falls back to a system font with different metrics. The regression is
subtle — simple formulas like x^2 + y^2 look nearly identical under
the system fallback — but anything using extensible delimiters
(matrices via \begin{bmatrix}, fractions, radicals) picks different
glyphs for the bracket/brace pieces and the vertical alignment breaks.
The root problem is that these files are duplicated between
node_modules/katex/dist/fonts/ and apps/meteor/public/fonts/. A proper
fix would be a build step that copies from node_modules at build time,
or a Meteor-side change to make the bundler process url() in imported
third-party CSS. Neither is in scope for this PR; restore the files as
they were so main runs unaffected.
The rocketchat.{eot,svg,ttf,woff} removal and the corresponding
rocketchat.font.css change stay — those were fallbacks in our own CSS,
controlled by the repo's browserslist, and every supported target has
WOFF2.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/.docker/Dockerfile.debian`:
- Line 42: Replace the invalid npm command in the Dockerfile: locate the line
containing "npm cache clear --force" and change it to "npm cache clean --force"
so the Debian Docker build uses the correct npm cache subcommand; ensure no
other occurrences of "npm cache clear" remain in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a0b79f1-70c6-4c91-8291-5a0e9d2ad4e9
⛔ Files ignored due to path filters (4)
apps/meteor/public/fonts/rocketchat.eotis excluded by!**/*.eotapps/meteor/public/fonts/rocketchat.svgis excluded by!**/*.svgapps/meteor/public/fonts/rocketchat.ttfis excluded by!**/*.ttfapps/meteor/public/fonts/rocketchat.woffis excluded by!**/*.woff
📒 Files selected for processing (3)
apps/meteor/.docker/Dockerfile.alpineapps/meteor/.docker/Dockerfile.debianapps/meteor/app/theme/client/rocketchat.font.css
✅ Files skipped from review due to trivial changes (1)
- apps/meteor/app/theme/client/rocketchat.font.css
📜 Review details
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/.docker/Dockerfile.debianapps/meteor/.docker/Dockerfile.alpine
📚 Learning: 2026-03-17T16:08:37.572Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 39690
File: packages/ui-voip/package.json:11-11
Timestamp: 2026-03-17T16:08:37.572Z
Learning: In `packages/ui-voip/package.json` (RocketChat/Rocket.Chat), the team deliberately chose to use `rm -rf dist` directly in the `"build"` script instead of `rimraf`, as they decided against introducing the `rimraf` dependency for this package. Do not flag `rm -rf dist` in the ui-voip build script as a cross-platform issue requiring rimraf.
Applied to files:
apps/meteor/.docker/Dockerfile.debian
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/.docker/Dockerfile.debian
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
apps/meteor/.docker/Dockerfile.debian
📚 Learning: 2026-04-14T23:23:18.990Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40159
File: packages/apps-engine/turbo.json:6-6
Timestamp: 2026-04-14T23:23:18.990Z
Learning: In RocketChat/Rocket.Chat's Turborepo setup, the workspace-root `package.json` is part of Turbo's global hash and does not need to be explicitly listed in a package-level turbo.json task's `inputs` array. Changes to the root `package.json` already bust the cache for all tasks via the global hash. Do not flag the absence of `$TURBO_ROOT$/package.json` in task-level inputs as a cache-invalidation issue.
Applied to files:
apps/meteor/.docker/Dockerfile.debian
🔇 Additional comments (2)
apps/meteor/.docker/Dockerfile.debian (1)
38-50: LGTM on the sourcemap cleanup approach.The
--omit=devflags correctly exclude devDependencies from production installs, and the selective sourcemap deletion preserves only what Meteor's source-map-support requires at runtime while removing ~260 MB of unnecessary files. The inline comments clearly document the rationale.apps/meteor/.docker/Dockerfile.alpine (1)
11-21: LGTM!The Alpine builder correctly uses
npm cache clean --force,--omit=devfor production installs, and the same sourcemap pruning strategy as the Debian variant. The multi-stage build ensures the cleanup reduces the final image size effectively.
Summary
A set of small, independent changes that together cut ~260 MB off the alpine Docker image (measured on
rocketchat/rocket.chat:develop).apps/meteor/packages/rocketchat-livechat/plugin/build.sh, delete*.mapafter copyingpackages/livechat/dist/intopublic/livechat/. Also prefix the widget'sbuildscript withyarn cleanso localdist/stops accumulating stale chunks (measured 500 MB → 18 MB on a fresh build locally).npm cache clean --forceto the alpine builder (was missing), and pass--omit=devto bothnpm installs in the debian variant (matches alpine).apps/meteor/public/fonts/rocketchat.{eot,svg,ttf,woff}symlinks and simplifyrocketchat.font.cssto only reference WOFF2 (browserslist is["last 2 versions", "Firefox ESR"]— every target supports WOFF2).npm install --omit=devin the builder stage, delete*.mapfiles underprograms/server/npm/node_modules/(third-party libs) andprograms/web.browser/. Maps underprograms/server/{app,packages,node_modules/meteor}are kept because Meteor'ssource-map-supportopens some of them at boot.Measured totals
Taken from flattened
rocketchat/rocket.chat:developimages (so layer whiteouts aren't hiding the real numbers):Smoke-tested: the resulting image boots against MongoDB with no
ENOENTon*.mapfiles (ServiceBroker starts, EventService runs).Method / reproducing
Why the narrow
findscopeAn earlier revision used a single
find /app/bundle -type f -name '*.map' -delete. That caused the container to crash at boot with:Meteor installs
source-map-supportand eagerly opens the meteor-packages bundle's.mapfile at startup. The scope is therefore restricted to paths Node never reads at runtime (server/npm/node_modules/**/*.map) plus the client bundle (web.browser/**/*.map).Test plan
yarn --cwd packages/livechat buildnow runsrimraf distfirst and produces a fresh, smallerdist/.meteor buildstill works;apps/meteor/public/livechat/no longer contains*.mapfiles.\begin{bmatrix}1&2\\3&4\end{bmatrix}) — font requests for/fonts/KaTeX_*must return 200 with valid font bytes, not the SPA index.html.docker build -f apps/meteor/.docker/Dockerfile.alpinesucceeds.docker build -f apps/meteor/.docker/Dockerfile.debiansucceeds.Task: ARCH-2099
Summary by CodeRabbit
Release Notes