Skip to content

Conversation

@tassoevan
Copy link
Contributor

@tassoevan tassoevan commented Nov 27, 2025

Proposed changes (including videos or screenshots)

It normalizes TypeScript and ESLint configurations for @rocket.chat/peggy-loader.

Issue(s)

ARCH-1898

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Chores
    • Restructured the build system and consolidated build configuration into a single pipeline.
    • Updated package entry points to reflect new build output structure.

✏️ Tip: You can customize this high-level summary in your review settings.

@tassoevan tassoevan added this to the 7.14.0 milestone Nov 27, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 27, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 7.14.0, but it targets 7.13.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 27, 2025

⚠️ No Changeset found

Latest commit: 8061437

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025

Walkthrough

The peggy-loader package build configuration is consolidated by removing separate ESM and CommonJS build targets, replacing them with a single unified tsc-based pipeline. Configuration files are simplified—Jest settings removed from ESLint, ignore patterns narrowed—and entry points updated to reflect the new unified output structure.

Changes

Cohort / File(s) Summary
TypeScript Configuration
packages/peggy-loader/tsconfig.json, packages/peggy-loader/tsconfig.build.json, packages/peggy-loader/tsconfig.cjs.json
Root tsconfig simplified to extend shared base and set module to esnext; new tsconfig.build.json added for build pipeline with outDir set to dist; tsconfig.cjs.json deleted
ESLint Configuration
packages/peggy-loader/.eslintignore, packages/peggy-loader/.eslintrc.json
Removed node_modules and dist exclusions from .eslintignore; removed Jest plugin and env from .eslintrc.json; updated ignorePatterns from **/dist to dist
Formatter & Git Configuration
packages/peggy-loader/.prettierignore, packages/peggy-loader/.prettierrc.js, packages/peggy-loader/.gitignore
Removed node_modules and dist from Prettier ignore; removed Prettier config re-export; removed dist from Git ignore
Package Metadata
packages/peggy-loader/package.json
Changed main from dist/cjs/index.js to dist/index.js and types from dist/esm/index.d.ts to dist/index.d.ts; consolidated build scripts to single tsc pipeline; removed separate esm/cjs build tasks
Source
packages/peggy-loader/src/index.ts
Whitespace and indentation formatting only

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Consolidation correctness: Verify the new single build output via tsconfig.build.json properly replaces the dual ESM/CommonJS approach
  • Entry point alignment: Confirm that dist/index.js and dist/index.d.ts are generated correctly by the new build pipeline
  • Jest removal impact: Ensure removal of Jest configuration from ESLint does not affect test execution or reporting
  • Git tracking change: Validate that tracking /dist in version control is intentional

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • ggazzo

Poem

🐰 A build consolidated clean and tight,
One tsc pipeline shining bright,
ESLint configs trimmed with care,
No more esm/cjs split to bear,
Peggy-loader bounces forward light! 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues check ❓ Inconclusive The PR references ARCH-1898 which matches the stated objective of reviewing build configuration, but the linked issue provides no specific coding requirements to validate against. Review the linked issue ARCH-1898 for specific requirements and acceptance criteria to fully validate that all coding objectives are met.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective of the PR - reviewing and normalizing the build configuration for the peggy-loader package.
Out of Scope Changes check ✅ Passed All changes focus on build configuration files (tsconfig, eslint, prettier, gitignore) and package.json scripts, which are in scope for a build configuration review.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/peggy-loader-build-configuration

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 66a2a20 and 8061437.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • packages/peggy-loader/.eslintignore (0 hunks)
  • packages/peggy-loader/.eslintrc.json (1 hunks)
  • packages/peggy-loader/.gitignore (0 hunks)
  • packages/peggy-loader/.prettierignore (0 hunks)
  • packages/peggy-loader/.prettierrc.js (0 hunks)
  • packages/peggy-loader/package.json (1 hunks)
  • packages/peggy-loader/src/index.ts (1 hunks)
  • packages/peggy-loader/tsconfig.build.json (1 hunks)
  • packages/peggy-loader/tsconfig.cjs.json (0 hunks)
  • packages/peggy-loader/tsconfig.json (1 hunks)
💤 Files with no reviewable changes (5)
  • packages/peggy-loader/.gitignore
  • packages/peggy-loader/.prettierrc.js
  • packages/peggy-loader/.eslintignore
  • packages/peggy-loader/.prettierignore
  • packages/peggy-loader/tsconfig.cjs.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • packages/peggy-loader/src/index.ts
🧠 Learnings (1)
📓 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.
🔇 Additional comments (6)
packages/peggy-loader/.eslintrc.json (1)

2-3: Jest removal is safe; the package has no test files and no Jest dependencies.

Verification confirms:

  • No test files exist in the peggy-loader package
  • No Jest field in package.json, no Jest in devDependencies
  • No env configuration in the current .eslintrc.json
  • The narrowed ignorePatterns from "**/dist" to "dist" appropriately reflects the unified build output structure

The configuration is correct as-is.

packages/peggy-loader/src/index.ts (1)

1-32: LGTM!

The formatting changes are consistent, and the implementation correctly uses Peggy's type system. The default 'commonjs' format aligns well with the simplified build output.

packages/peggy-loader/tsconfig.json (1)

1-7: LGTM!

Clean configuration that properly extends the shared base config. The minimal override for module: "esnext" keeps this maintainable while leveraging centralized settings.

packages/peggy-loader/tsconfig.build.json (1)

1-9: LGTM!

Well-structured build configuration that properly separates development and build concerns. Enabling declaration, declarationMap, and sourceMap is appropriate for a published package.

packages/peggy-loader/package.json (2)

24-34: LGTM on the build configuration consolidation.

The unified build pipeline with a single tsc compilation step is cleaner than separate ESM/CJS targets. Entry points correctly reference the new dist/ output structure.


10-18: No action needed—repository URLs are intentionally correct.

The homepage, bugs, and repository URLs correctly reference RocketChat/fuselage because peggy-loader (along with message-parser and fuselage-ui-kit) is part of a suite of packages maintained in the fuselage repository. These packages exist in the Rocket.Chat monorepo as workspace dependencies but maintain cross-references to their canonical source in fuselage. This pattern is consistent across multiple packages in the codebase and is intentional.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tassoevan tassoevan force-pushed the chore/peggy-loader-build-configuration branch from ce21d60 to 8061437 Compare November 27, 2025 18:01
@github-actions
Copy link
Contributor

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.2GiB 1.2GiB +12MiB
rocketchat 358MiB 346MiB +12MiB
omnichannel-transcript-service 132MiB 132MiB -806B
queue-worker-service 132MiB 132MiB -105B
ddp-streamer-service 127MiB 127MiB -683B
account-service 114MiB 114MiB -2.6KiB
stream-hub-service 111MiB 111MiB -1.4KiB
authorization-service 111MiB 111MiB -586B
presence-service 111MiB 111MiB -585B

📊 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/27 18:23 (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]
Loading

Statistics (last 7 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.2GiB
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-37629
  • Baseline: develop
  • Timestamp: 2025-11-27 18:23:19 UTC
  • Historical data points: 7

Updated: Thu, 27 Nov 2025 18:23:19 GMT

@codecov
Copy link

codecov bot commented Nov 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.84%. Comparing base (66a2a20) to head (8061437).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #37629      +/-   ##
===========================================
+ Coverage    68.81%   68.84%   +0.03%     
===========================================
  Files         3361     3361              
  Lines       114277   114277              
  Branches     20618    20618              
===========================================
+ Hits         78634    78674      +40     
+ Misses       33550    33510      -40     
  Partials      2093     2093              
Flag Coverage Δ
e2e 57.34% <ø> (+0.02%) ⬆️
e2e-api 43.23% <ø> (+1.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tassoevan tassoevan marked this pull request as ready for review November 27, 2025 19:25
@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Nov 27, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Nov 27, 2025
@kodiakhq kodiakhq bot merged commit f0531bc into develop Nov 27, 2025
53 checks passed
@kodiakhq kodiakhq bot deleted the chore/peggy-loader-build-configuration branch November 27, 2025 19:32
@dougfabris dougfabris modified the milestones: 7.14.0, 8.0.0 Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants