Skip to content

chore: dedupe telemetrymap types in core services - #41771

Open
kushals256 wants to merge 1 commit into
RocketChat:developfrom
kushals256:chore/dedupe-telemetry-types
Open

chore: dedupe telemetrymap types in core services#41771
kushals256 wants to merge 1 commit into
RocketChat:developfrom
kushals256:chore/dedupe-telemetry-types

Conversation

@kushals256

@kushals256 kushals256 commented Aug 13, 2026

Copy link
Copy Markdown

fixes #39994

telemetrymap and telemetryevents were duplicated in itelemetryevent.ts. the file already had a todo pointing at rest typings. core services depends on rest typings, so i imported the types from there and re exported them.

no runtime change.

@dionisio-bot

dionisio-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

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

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@CLAassistant

CLAassistant commented Aug 13, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0bfb6ec6-8405-4a1a-b14d-d032f6774f1d

📥 Commits

Reviewing files that changed from the base of the PR and between 5deefe2 and 48f0b18.

📒 Files selected for processing (1)
  • packages/core-services/src/types/ITelemetryEvent.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/core-services/src/types/ITelemetryEvent.ts
packages/**

📄 CodeRabbit inference engine (CLAUDE.md)

Shared libraries belong in packages/, while other services belong in apps/ and ee/.

Files:

  • packages/core-services/src/types/ITelemetryEvent.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
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.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40469
File: packages/rest-typings/src/v1/users.ts:337-337
Timestamp: 2026-05-11T23:15:03.372Z
Learning: In RocketChat/Rocket.Chat, REST endpoint typings in `packages/rest-typings/src/v1/users.ts` (and broadly across the codebase) reference `IUser` field types directly (e.g. `IUser['statusExpiresAt']`) rather than serialized equivalents like `string`. Suggesting a change to serialized types (e.g. `string`) for a single field is inconsistent with this pattern; any such refactor should be done codebase-wide.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 41205
File: apps/meteor/app/apps/server/converters/users.ts:54-54
Timestamp: 2026-07-16T16:21:13.272Z
Learning: In `apps/meteor/app/apps/server/converters/users.ts`, both the Apps-Engine `IAppsUser` type and Rocket.Chat core `IUser` type use the correctly spelled `utcOffset` property. The historical `utfOffset` spelling was only a prior converter implementation detail and must not be treated as a supported input compatibility contract.
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40186
File: apps/meteor/app/apps/server/bridges/uiInteraction.ts:2-2
Timestamp: 2026-05-06T20:48:08.244Z
Learning: In the RocketChat/Rocket.Chat repository, Meteor's bundler does not respect the `exports` keyword in `package.json` files. Deep imports (e.g., `rocket.chat/apps/dist/server/bridges/UiInteractionBridge`) must be used instead of relying on `exports` subpath mappings. Do not suggest adding `exports` map entries to packages consumed by Meteor (e.g., `packages/apps/package.json`) as a fix for deep imports.
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 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:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 Learning: 2026-05-11T23:15:03.372Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40469
File: packages/rest-typings/src/v1/users.ts:337-337
Timestamp: 2026-05-11T23:15:03.372Z
Learning: In RocketChat/Rocket.Chat, REST endpoint typings in `packages/rest-typings/src/v1/users.ts` (and broadly across the codebase) reference `IUser` field types directly (e.g. `IUser['statusExpiresAt']`) rather than serialized equivalents like `string`. Suggesting a change to serialized types (e.g. `string`) for a single field is inconsistent with this pattern; any such refactor should be done codebase-wide.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.

Applied to files:

  • packages/core-services/src/types/ITelemetryEvent.ts
🔇 Additional comments (1)
packages/core-services/src/types/ITelemetryEvent.ts (1)

1-3: LGTM!


Walkthrough

Core services now re-export TelemetryEvents and TelemetryMap from @rocket.chat/rest-typings instead of defining duplicate local types.

Changes

Telemetry type consolidation

Layer / File(s) Summary
Re-export shared telemetry types
packages/core-services/src/types/ITelemetryEvent.ts
The file imports and re-exports TelemetryEvents and TelemetryMap from @rocket.chat/rest-typings. Local duplicate definitions and the duplication comment are removed.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Mergeability Score: ⚪ Minimal · up to 48f0b

This localized type-only change introduces no runtime behavior changes or actionable merge-blocking risk and is ready to merge after normal checks.

Suggested labels: type: chore

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR imports and re-exports TelemetryMap and TelemetryEvents from rest-typings, satisfying issue #39994.
Out of Scope Changes check ✅ Passed The changes are limited to removing duplicate telemetry type definitions and adding the required import and re-export.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing duplicate telemetry map types from core services.

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.

@kushals256 kushals256 changed the title chore: dedupe TelemetryMap types in core-services chore: dedupe telemetrymap types in core services Aug 13, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: remove duplicate TelemetryMap and TelemetryEvents types in core services

2 participants