chore(ui-contexts): Review build configuration #37598
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughConsolidates and standardizes the build configuration for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/24 17:37 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 7 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 24 Nov 2025 17:37:30 GMT |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37598 +/- ##
========================================
Coverage 68.84% 68.84%
========================================
Files 3361 3361
Lines 114342 114342
Branches 20619 20619
========================================
+ Hits 78714 78715 +1
- Misses 33530 33537 +7
+ Partials 2098 2090 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui-contexts/package.json (1)
10-17: Consolidate duplicate test scripts.Both
"test"and"testunit"scripts on lines 15–16 are identical (both run"jest"). Typically, one script name suffices unless there is a semantic distinction (e.g., unit tests vs. integration tests). Having duplicate scripts can cause confusion during maintenance.Consider removing one script or clarifying their purpose if they serve different test suites. If there is no distinction, keep only
"test".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/ui-contexts/.eslintrc.json(1 hunks)packages/ui-contexts/package.json(2 hunks)packages/ui-contexts/tsconfig.build.json(1 hunks)packages/ui-contexts/tsconfig.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.039Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.039Z
Learning: Applies to **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
packages/ui-contexts/tsconfig.build.json
📚 Learning: 2025-11-24T17:08:17.039Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.039Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/ui-contexts/tsconfig.build.jsonpackages/ui-contexts/tsconfig.json
📚 Learning: 2025-11-24T17:08:17.039Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.039Z
Learning: Applies to **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
packages/ui-contexts/tsconfig.json
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/ui-contexts/package.json
🔇 Additional comments (7)
packages/ui-contexts/.eslintrc.json (2)
2-2: Verify that removing react-hooks ESLint recommendation is intentional.The package is React-based and includes
eslint-plugin-react-hooksin devDependencies. Removing"plugin:react-hooks/recommended"disables critical ESLint rules for React Hook compliance (e.g., dependency array validation, hook call conditions). This should be an active, deliberate decision.Can you confirm that this removal is intentional and that the package does not need these hook-related lint rules?
3-3: Verify that ignorePatterns scope change does not miss nested dist directories.The ignorePatterns changed from
"**/dist"(recursive, matches all dist directories) to"dist"(top-level only). If this package has nested directories with dist folders, they will no longer be ignored by ESLint.Please verify that there are no nested dist directories in this package that should be ignored, or update the pattern to
**/distif recursive matching is needed.packages/ui-contexts/tsconfig.json (2)
4-4: Clarify the purpose of rootDirs including both source and root directories.Using
rootDirswith both"./src"and"./"is unconventional. TherootDirsoption is primarily for output mapping; for source organization, a singlerootDirpointing to"./src"is typical. Including"./"(root) alongside"./src"may cause unexpected module resolution behavior or allow imports from unintended locations.Can you explain the intent behind this rootDirs configuration? If the goal is to allow imports from both src and root, consider whether this is the desired module resolution behavior, or if a simpler single
rootDirwould suffice.
7-7: Move jest.config.ts inclusion to build-time configuration, not main TypeScript config.The
jest.config.tsshould not typically be included in the main project's TypeScript compilation (includearray). Test configuration files are usually only type-checked during development or as part of tooling, not bundled as part of the project. The newtsconfig.build.jsonfile already excludes this file, which is correct for production builds, but including it here in the main tsconfig may cause unexpected behavior.Please verify whether
jest.config.tsshould be in the maintsconfig.jsonor if it should be excluded here (and only handled separately during development/testing).packages/ui-contexts/tsconfig.build.json (1)
1-4: Build configuration properly excludes test artifacts; however, verify against parent tsconfig patterns.The new build configuration correctly excludes test files (
.spec.ts,.spec.tsx) and jest configuration. However, since this extendstsconfig.json, it inherits thejest.config.tsinclusion from the parent config and then explicitly excludes it here—a redundant pattern. This becomes consistent only ifjest.config.tsis also removed from the maintsconfig.json.Please verify that the parent
tsconfig.jsonis updated to exclude or removejest.config.tsfrom itsincludearray, or confirm that the dual inclusion/exclusion is intentional for your build workflow.packages/ui-contexts/package.json (2)
4-9: Contradictory package configuration: marked private but declares public package fields.The package is marked
"private": trueon line 4, yet lines 5–9 add public package fields (main,typings,files). Private packages in monorepos typically do not need these fields, as they are not published to npm. If this package is intended to remain private (not published), these fields are unnecessary. If it will be published, remove or update the"private"flag.Please clarify the publication intent: Is this package private to the monorepo, or should it be published to npm?
11-12: Verify that build and dev scripts correctly reference the new tsconfig.build.json.The
buildscript (line 11) uses-p tsconfig.build.jsonto reference the new build-specific configuration, which correctly excludes test files. Thedevscript (line 12) uses watch mode with the same config. Confirm that this aligns with your monorepo's build expectations and that the TypeScript configuration chain (base → build) works as intended.Run a build to verify that the build output is generated correctly in
./distand contains only production code (no test files or jest config).
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
It normalizes TypeScript and ESLint configurations for
@rocket.chat/ui-contexts.Issue(s)
ARCH-1888
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.