chore!: remove orphan Meteor (DDP) methods scheduled for 9.0.0#40656
Draft
ggazzo wants to merge 8 commits into
Draft
chore!: remove orphan Meteor (DDP) methods scheduled for 9.0.0#40656ggazzo wants to merge 8 commits into
ggazzo wants to merge 8 commits into
Conversation
- scripts/audit-ddp-methods.mjs: enumerates Meteor.methods registrations,
callers, and REST replacements, producing docs/ddp-audit.{json,md}.
- scripts/remove-orphan-ddp-methods.mjs: removes orphan method keys from
Meteor.methods({...}) blocks; if the block becomes empty, drops the
whole statement. Uses a skip-list for methods reachable via admin
MethodActionInput dynamic dispatch.
- scripts/cleanup-after-orphan-removal.mjs: prunes the now-unused imports
in files we just edited, deletes files that no longer contain any code
besides imports and declare-module blocks, and removes the matching
side-effect imports from barrel index.ts files.
Removes 94 Meteor method registrations that had no callers anywhere in the repository — they were marked for removal in 9.0.0 by the deprecation logging shipped earlier on develop. Files that ended up containing only imports and declare-module blocks after the removal were deleted, and matching side-effect imports were stripped from sibling index.ts barrels. The removal keeps a skip-list of methods that are still reachable indirectly: admin settings buttons that dispatch via useMethod(value) in MethodActionInput.tsx, and a handful of cloud:* methods invoked from admin actions. Those will be revisited once their callers move to REST. Removed by scripts/remove-orphan-ddp-methods.mjs + scripts/cleanup-after-orphan-removal.mjs from docs/ddp-audit.json. BREAKING CHANGE: 94 DDP methods previously registered are no longer callable. Any DDP-only client that still relies on Meteor.call(<name>) for these methods must migrate to the corresponding REST endpoint (see docs/ddp-audit.md for the mapping) or, where no REST equivalent exists, drop usage.
Contributor
|
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 |
🦋 Changeset detectedLatest commit: 8407e3c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
2 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-9.0.0 #40656 +/- ##
=================================================
+ Coverage 69.65% 69.71% +0.05%
=================================================
Files 3322 3314 -8
Lines 122579 122318 -261
Branches 21867 21795 -72
=================================================
- Hits 85379 85270 -109
+ Misses 33860 33715 -145
+ Partials 3340 3333 -7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The first pass of the orphan removal triggered three classes of CI
failures:
1. `@typescript-eslint/no-unused-vars` errors where the import-pruner
removed type imports that were still referenced inside the
`declare module '@rocket.chat/ddp-client' { interface ServerMethods
{ ... } }` augmentation blocks. The pruner was stripping declare
module content before checking name usage, so types like
`IImport`, `ICreatedRoom`, `IMessageSearchProvider`,
`RoomType`, `RocketChatRecordDeleted`, and `WithId` were
incorrectly marked unused.
2. `@typescript-eslint/no-unused-vars` errors where the interface
ServerMethods entries for the removed methods were still pointing
at types we had already imported. The new
scripts/strip-orphan-interface-entries.mjs walks each declare
module block in step with the removal plan and drops the matching
interface entries (and the whole declare module block if it
becomes empty), keeping the implementation removal and the type
surface in sync. A skip-list inside the script restores entries
for the 8 methods that turned out to have callers we missed.
3. `error TS2345: Argument of type '...' is not assignable to
parameter of type 'keyof ServerMethods'` for methods that the
audit reported as orphan but were called through dynamic dispatch
the literal-string scanner couldn't see:
- `permissions/get`, `rooms/get`, `subscriptions/get`,
`public-settings/get`, `private-settings/get` are called
via a template literal in
apps/meteor/client/lib/cachedStores/CachedStore.ts as
`sdk.call(\`${this.name}/get\`)`.
- `samlLogout` is called via `sdk\n.call('samlLogout', ...)`
in apps/meteor/client/meteor/login/saml.ts — the audit's
`\bsdk\.` regex didn't tolerate whitespace between
`sdk` and `.call`.
- `blockUser` and `unblockUser` are called via a conditional
`useMethod(isUserBlocked ? 'unblockUser' : 'blockUser')` in
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useBlockUserAction.ts,
which the scanner can't reach.
Restoring those 8 implementations and adjusting the matching barrel
imports puts the runtime contract back together. The scripts'
detection patterns will be hardened in a follow-up.
Also drops apps/meteor/app/push/server/methods.ts and its
`import './methods'` from `apps/meteor/app/push/server/index.ts`:
the file's last DDP entry was removed in an earlier pass, leaving the
file with nothing but an unused `PushUpdateOptions` alias and an
orphan interface declaration.
apps/meteor/server/importPackages.ts:71 imports ../app/meteor-accounts-saml/server, so the directory needs an index.ts that loads the side-effect modules. The previous cleanup pass deleted the barrel when it looked dead, breaking lint and the runtime require.
ggazzo
added a commit
that referenced
this pull request
May 22, 2026
The deprecation logger throws in TEST_MODE
(apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts:118), so a
deprecation log on a method that's still invoked by integration or
unit tests turns those tests into failures with messages like:
Error: The method "addUsersToRoom" is deprecated and will be
removed on version 9.0.0
That's the intended dev-loop guard rail, but it means this PR can
only carry deprecation calls on methods with **no callers**.
Removes the 83 logger calls (and their now-unused imports) that
targeted methods listed in audit.used. They will be revisited as part
of the caller-migration PR #40659 — once each callsite moves to the
matching REST endpoint, the DDP method can either get a deprecation
log here or be removed outright in #40656.
What remains in this PR: 71 deprecation calls, all on orphan methods
without a REST replacement.
2 tasks
Lists the 87 Meteor (DDP) methods this PR removes, with the matching REST endpoint where one exists. Major bump on @rocket.chat/meteor flags the breaking-change semantics for external DDP/SDK clients.
The tagged describe blocks in tests/end-to-end/api/methods.ts that
exercised method.call/:method directly against the removed methods
no longer have anything to test - the implementations are gone in
this PR. Removed the four blocks (1005 lines):
- [@cleanRoomHistory]
- [@getUsersOfRoom]
- [@updateMessage]
- [@setUserActiveStatus]
Two unrelated test files used `saveUserPreferences` as a setup step
before exercising a different feature. Migrated those callsites to
POST /v1/users.setPreferences so the test still does what it intended
once the DDP method is gone:
- tests/end-to-end/api/direct-message.ts: setting emailNotificationMode
before creating a DM.
- tests/end-to-end/api/teams.ts: setting emailNotificationMode before
inviting the user to a team.
The `methodCall` import in direct-message.ts was the only remaining
reference; dropped it to satisfy the unused-vars rule.
ggazzo
added a commit
that referenced
this pull request
May 25, 2026
… proxy These 5 methods were classified as orphan by the static-string caller audit but are actually invoked at runtime from API tests through the `/v1/method.call/:method` REST proxy. Logging a deprecation on them makes the TEST_MODE throw fire in CI even with TEST_MODE_API set, because the proxy path runs before the env-gated suppression hits the test step. Removes the logger calls and now-unused imports for: - cleanRoomHistory - getUsersOfRoom - readMessages - setUserActiveStatus - updateMessage These methods are removed entirely in #40656 (release-9.0.0); deprecation logging on develop is therefore redundant. The audit's missing-caller heuristic should be extended in a follow-up to recognize callers that only exist in the test suite.
UI tests cover the forgot-password flow which triggers `sendForgotPasswordEmail` through Meteor's `Accounts.forgotPassword` client API — the call chain is invisible to the audit's static string-literal scan. Restores the method file from origin/release-9.0.0, drops the entry from the changeset (count 87 -> 86), and adds it to the NEVER_REMOVE skip list in scripts/remove-orphan-ddp-methods.mjs alongside the other dynamic-dispatch survivors so a future audit run doesn't try to remove it again.
The [@getReadReceipts] EE test used readMessages as a fixture step to simulate the invited user reading the main room before checking that both sender and recipient receipts show up. readMessages is removed in this PR; the test now POSTs /v1/subscriptions.read instead, which is the supported REST equivalent.
|
Member
|
/layne exception-approve LAYNE-6ceb2179c9a0541a reason: hardcoded command - it's not generated dynamically and poses no risk as is |
|
✅ Exception recorded for LAYNE-6ceb2179c9a0541a by @julio-rocketchat: "hardcoded command - it's not generated dynamically and poses no risk as is". Re-running scan... |
ggazzo
added a commit
that referenced
this pull request
May 25, 2026
… proxy These 5 methods were classified as orphan by the static-string caller audit but are actually invoked at runtime from API tests through the `/v1/method.call/:method` REST proxy. Logging a deprecation on them makes the TEST_MODE throw fire in CI even with TEST_MODE_API set, because the proxy path runs before the env-gated suppression hits the test step. Removes the logger calls and now-unused imports for: - cleanRoomHistory - getUsersOfRoom - readMessages - setUserActiveStatus - updateMessage These methods are removed entirely in #40656 (release-9.0.0); deprecation logging on develop is therefore redundant. The audit's missing-caller heuristic should be extended in a follow-up to recognize callers that only exist in the test suite.
ggazzo
added a commit
that referenced
this pull request
May 25, 2026
… proxy These 5 methods were classified as orphan by the static-string caller audit but are actually invoked at runtime from API tests through the `/v1/method.call/:method` REST proxy. Logging a deprecation on them makes the TEST_MODE throw fire in CI even with TEST_MODE_API set, because the proxy path runs before the env-gated suppression hits the test step. Removes the logger calls and now-unused imports for: - cleanRoomHistory - getUsersOfRoom - readMessages - setUserActiveStatus - updateMessage These methods are removed entirely in #40656 (release-9.0.0); deprecation logging on develop is therefore redundant. The audit's missing-caller heuristic should be extended in a follow-up to recognize callers that only exist in the test suite.
ggazzo
added a commit
that referenced
this pull request
May 26, 2026
The deprecation logger throws in TEST_MODE
(apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts:118), so a
deprecation log on a method that's still invoked by integration or
unit tests turns those tests into failures with messages like:
Error: The method "addUsersToRoom" is deprecated and will be
removed on version 9.0.0
That's the intended dev-loop guard rail, but it means this PR can
only carry deprecation calls on methods with **no callers**.
Removes the 83 logger calls (and their now-unused imports) that
targeted methods listed in audit.used. They will be revisited as part
of the caller-migration PR #40659 — once each callsite moves to the
matching REST endpoint, the DDP method can either get a deprecation
log here or be removed outright in #40656.
What remains in this PR: 71 deprecation calls, all on orphan methods
without a REST replacement.
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
2 participants
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.
Summary
Removes 94 Meteor (DDP) method registrations that have no callers anywhere in this repository. These were already flagged for removal in 9.0.0 on develop via
methodDeprecationLogger.method(name, '9.0.0', ...)(#40654).Method files that ended up containing only imports + a
declare module '@rocket.chat/ddp-client'block after the removal are deleted entirely, and matching side-effect imports are stripped from siblingindex.tsbarrels.Counters
How
Three scripts under
scripts/:audit-ddp-methods.mjs— enumerates Meteor methods + callers + REST routes, producesdocs/ddp-audit.{json,md}.remove-orphan-ddp-methods.mjs— strips orphan keys fromMeteor.methods({...})blocks; if the block becomes empty, drops the whole statement.cleanup-after-orphan-removal.mjs— prunes unused imports, deletes files that are now nothing-but-imports +declare module, removes barrel side-effect imports.Skip list (intentionally kept)
The following methods are reachable indirectly via the admin settings UI (
apps/meteor/client/views/admin/settings/Setting/inputs/MethodActionInput.tsx:12callsuseMethod(value)wherevalueis the setting's stored method name) or via cloud admin actions. They look orphan to the string-grep audit but are still callable, so they are excluded:OEmbedCacheCleanup,restart_server,push_test,crowd_sync_users,crowd_test_connection,loadLocale,resetIrcConnection,checkFederationConfiguration,removeSlackBridgeChannelLinks,cloud:checkRegisterStatus,cloud:registerWorkspace,cloud:checkUserLoggedIn,cloud:logout,cloud:finishOAuthAuthorization,cloud:getOAuthAuthorizationUrl.Risk
This is the destructive half of the DDP-removal effort started on develop. The audit only matches literal
Meteor.call('name', ...)strings — anything calling these methods with a dynamic name, from the mobile SDK, or from a third-party client that talks DDP directly, will start failing asmethod-not-found. Reviewers should flag any method below that they know is invoked externally.declare module '@rocket.chat/ddp-client' { interface ServerMethods { ... } }augmentations for the removed methods are still present in some surviving files (the type entries are not load-bearing now that no implementation exists). They can be swept up in a follow-up if desired.Test plan
apps/meteorbuilds and typechecks