refactor: incrementally remove Meteor deps from the frontend#40268
Draft
refactor: incrementally remove Meteor deps from the frontend#40268
Conversation
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 |
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 |
|
ggazzo
force-pushed
the
worktree-remove-meteor-frontend
branch
from
2d71f41 to
dd3dac4
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40268 +/- ##
===========================================
+ Coverage 69.80% 69.85% +0.05%
===========================================
Files 3296 3291 -5
Lines 119173 118993 -180
Branches 21435 21443 +8
===========================================
- Hits 83183 83119 -64
+ Misses 32684 32590 -94
+ Partials 3306 3284 -22
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
force-pushed
the
worktree-remove-meteor-frontend
branch
5 times, most recently
from
01d8539 to
16fe4f8
Compare
ggazzo
changed the title
VideoRecorder used meteor/reactive-var for cameraStarted, but the React consumer never subscribed reactively (no useTracker / useSyncExternalStore), so the send button only re-evaluated its disabled state by accident. Drop meteor/reactive-var from this module and restore reactivity using @rocket.chat/emitter — the pattern already used elsewhere in the client — plus a useSyncExternalStore-based useVideoRecorderCameraStarted hook so VideoMessageRecorder re-renders when the camera starts/stops. The unused private recording flag is removed; recordingAvailable stays since it gates the first-data-available transition internally.
Replace Meteor.absoluteUrl(path) with new URL(path, ROOT_URL), resolving the worker script via the standard URL API. Preserves subpath-deployment behavior since __meteor_runtime_config__.ROOT_URL already carries the configured base.
AutoTranslate.init() is idempotent and the module is imported through the client startup chain, so wrapping in Meteor.startup was a no-op in practice.
Meteor.Error was only used to narrow the .error string on a caught result; replace with an inline structural cast.
Meteor._localStorage wraps window.localStorage with a try/catch for browsers that throw on access (Safari private mode, sandboxed iframes). Inline the same wrapper locally so the util no longer needs the Meteor import.
timeAgo is only called from React components, which do not establish a Tracker computation, so Tracker.nonreactive around getUserPreference never has an outer autorun to opt out of — the wrapper is a no-op.
…stic call Meteor.methods registered setReaction as a client-side stub so that Meteor.callAsync would run it locally for optimistic UI. Move that logic into an exported runOptimisticSetReaction function and call it directly from processSetReaction (the sole caller), then REST-call the server method. Drops meteor/meteor from this module and removes the side-effectful barrel that only existed to register the stub.
…stic call Same approach as setReaction: convert the client-side Meteor.methods stub into an exported runOptimisticSendMessage function and invoke it from flows/sendMessage before the sdk.call. The slash-command asciiart callers (/tableflip, /lenny, etc.) no longer get an optimistic insert — those flows now wait on the server roundtrip like the REST /chat.sendMessage callers always did.
This file only exists to point Meteor.absoluteUrl at the page's base URI. Once the webapp/DDP layer is gone, Meteor.absoluteUrl itself goes away and so does this override — leave a TODO to remove them together.
The Tracker.autorun wrapping the inquiries fetch never re-ran in practice: its only dependency, settings.peek, is non-reactive by design, so changes to Livechat_guest_pool_max_number_incoming_livechats_displayed weren't triggering a refetch. Preserve the intended reactivity without Meteor: fetch once on subscribe, then observe the setting via settings.observe and refetch when it changes. The teardown returned by subscribe now unobserves.
…dule
Two Tracker.autorun calls are replaced with direct Zustand/settings
subscriptions:
- The module-level cache of the current user's language/username moves
from Tracker.autorun(watchUser) to plain subscriptions on userIdStore
and the Users store, triggering refreshUserCache on change.
- init() no longer uses Tracker. It subscribes to userIdStore,
settings.observe('AutoTranslate_Enabled'), and
PermissionsCachedStore.useReady, and retries tryLoad on any of those
signals until the gate (logged in + setting on + permission granted)
is passed. Once passed, the subscriptions tear down — matching the
original 'computation.stop()' one-shot semantics.
Also drops the Meteor.startup wrapper: importPackages loads this module
after startup has already fired, so it ran synchronously anyway.
…ter + hook Per-room history state (hasMore, hasMoreNext, isLoading, unreadNotLoaded, firstUnread) moves from ReactiveVar wrappers to plain fields on a RoomHistoryState object. The manager emits 'state:<rid>' whenever a field is patched through the new updateRoom method, and exposes a useRoomHistoryState(rid, selector) hook built on useSyncExternalStore. RoomProvider and useUnreadMessages switch from useReactiveValue to the new hook, and readStateManager uses direct reads plus updateRoom for its internal writes. Tracker.nonreactive wrappers become plain property reads, and Tracker.afterFlush in the scroll handler becomes queueMicrotask — no consumer was relying on Tracker flush ordering.
Both useCorsSSLConfig (patches Meteor.absoluteUrl.defaultOptions.secure) and oauthRedirectUri (monkey-patches meteor/oauth for pre-2.3 clients) only exist to bridge Meteor behaviour. They'll be deleted alongside the webapp/DDP layer — leave TODOs pointing to that.
Meteor.Error/Meteor.TypedError were only used to widen the callback error type from loginWithToken. Replace with an inline structural alias so useIframe no longer imports meteor/meteor.
Replace Meteor.absoluteUrl with new URL against __meteor_runtime_config__.ROOT_URL, same treatment as AudioEncoder. Preserves subpath-deployment behaviour.
Accounts.storageLocation is typed as Window['localStorage'] in the
project externals, and the key being read ('e2e.randomPassword') is
not an Accounts-managed key. Use window.localStorage directly.
The Tracker.autorun wrapping the sendMessage call had no reactive dependencies — openedRoom is a closure value from useOpenedRoom, not a reactive source — so the autorun ran exactly once and c.stop() was never reached on mismatch. Replace with a plain conditional, which preserves the actual behaviour. Also add the missing @rocket.chat/random import that was relying on an implicit global.
…vider The Session context only needs a string-keyed store plus per-key subscriptions. Swap Meteor's Session (plus createReactiveSubscriptionFactory, which wrapped Session.get in a Tracker.autorun) for a plain Map backed by @rocket.chat/emitter. Equality check mirrors Meteor's Session.set — no emit when the value hasn't changed — so useSession consumers (useUnread, AuthenticationCheck, SidebarToggler, etc.) re-render only on actual writes. This was the last client-side import of meteor/session.
Inline LoginWithExternalServiceOptions locally (same shape as @types/meteor defines) so the interface file no longer imports meteor/meteor. The type is exported from this module for any future consumer that wants to share it; existing OAuth providers still reference Meteor.LoginWithExternalServiceOptions directly and will be migrated alongside the rest of the Accounts layer.
The Tracker.autorun wrapped a setTimeout body that reads no reactive state and immediately calls c.stop(). Replace with a plain setTimeout; cleanup is clearTimeout.
The module is loaded through importPackages after Meteor has already started, so the outer Meteor.startup call fires synchronously — it's purely noise. onLoggedIn stays; the actual side effects (two notify-user stream subscriptions) run module-top-level now.
The Tracker.autorun/afterFlush that waited for watchUserId to become truthy is replaced with a userIdStore.subscribe that tears itself down once a uid is seen. onLoggedIn (for the initial roles.list fetch) and the event handlers are unchanged; the outer Meteor.startup wrapper is dropped as a no-op.
The Tracker.autorun gated the autotranslate streamMessage handler on
AutoTranslate_Enabled (reactive via settings.watch) and hasPermission
('auto-translate') — the latter reactive through the watch() helper's
Tracker reads of Users/Permissions.
Replace with direct subscriptions on settings.observe, PermissionsCachedStore.useReady
and Users.use that re-run applyAutoTranslateStreamHandler. Same
toggle-on/toggle-off semantics, no Tracker dependency.
Accounts.onLogin/onLogout catch every userId transition that the previous Tracker.autorun on Accounts.connection.userId() did — including cross-tab login, which is delivered through the same setUserId code path and therefore fires the Accounts handlers in the receiving tabs. Sync userIdStore once at module load for the 'already logged in' case, then keep it in sync via the Accounts handlers.
The original Tracker.autorun gated user-data sync on watchUserId() + Meteor.status().connected + !Meteor.loggingIn() before fetching. onLoggedIn fires after login completes, when the connection is necessarily up and loggingIn is false, so those two Meteor-specific gates become redundant. Use onLoggedIn for the initial sync + utcOffset update, subscribe to the Users store to emit 'status-changed' on subsequent server-side status changes (delivered via the user stream), and Accounts.onLogout to reset local state. No Tracker, no Meteor.status, no Meteor.loggingIn.
businessHourManager holds a single behavior instance with no reactive store under it, so the Tracker.autorun inside useReactiveValue never re-fired. Inline the plain read.
useMessageComposerIsReadOnly, useFileUploadDropTarget, and ReactionMessageAction
all wrapped a roomCoordinator.readOnly(room, user) call in useReactiveValue
to pick up changes in the post-readonly permission.
Replace with usePermission('post-readonly', room._id) as an explicit
dependency of a plain useMemo — when the permission flips, the memo
re-runs and roomCoordinator.readOnly returns the up-to-date value
(hasPermission underneath is Tracker-aware but returns the current
value when no autorun is active).
The canSendMessage room directive reads Subscriptions.state.count, which is a Zustand snapshot — Tracker.autorun inside useReactiveValue never registered a dep on it, so the previous reactivity was effectively broken. Subscribe properly via useSyncExternalStore on Subscriptions.use so canSend updates when the user joins or leaves the room.
ggazzo
force-pushed
the
worktree-remove-meteor-frontend
branch
from
a820570 to
1c213a3
Compare
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
1 participant
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
Incrementally removes Meteor runtime imports from the client without changing behaviour. 19 small, independently reviewable commits covering:
Reactivity migrations
ReactiveVar→@rocket.chat/emitter+useSyncExternalStorehook) — also fixes a latent bug where the send button's disabled state was never re-evaluated reactively.ReactiveVar+Tracker.nonreactive+Tracker.afterFlush→ plain fields patched viaupdateRoom, newuseRoomHistoryState(rid, selector)hook) — last client-sideReactiveVar.meteor/session+createReactiveSubscriptionFactory→Map+ Emitter with value-equality check) — eliminatesmeteor/sessionfrom the client entirely.Tracker.autorunwithsettings.peekinside (which never registered a dep — quietly broken) →settings.observeso setting changes actually refetch inquiries.Tracker.autorunreplaced with direct Zustand/settings subscriptions; keeps the one-shot "wait for gate, fetch, unsubscribe" semantics.Tracker.nonreactivewrapper was a no-op outside any autorun; removed.Tracker.autorunover a non-reactive closure value; replaced with plain conditional.Method stubs → explicit optimistic updates
Meteor.methodsstub replaced withrunOptimisticSendMessage(message)called explicitly beforesdk.call. Fixed a CI-caught subtlety where the original mutation pattern relied on Meteor cloning stub args; the new version writes an optimistic copy and leaves the outgoing message pristine.Trivial drops
AudioEncoder.ts:Meteor.absoluteUrl(path)→new URL(path, __meteor_runtime_config__.ROOT_URL)(same forroomCoordinator).getConfig.ts:Meteor._localStorage→window.localStoragewith the same try/catch wrapper.slashcommands-join,useIframe,IOAuthProvider: Meteor type-only imports (e.g.,Meteor.Error,Meteor.LoginWithExternalServiceOptions) inlined as local structural types.RoomE2EESetup:Accounts.storageLocationis typed asWindow['localStorage']in project externals and the key being read isn't Accounts-managed, so usewindow.localStoragedirectly.actionButton(autotranslate):Meteor.startup(…)wrapper was a no-op post-bootstrap; removed.TODOs for overrides that leave with DDP
client/meteor/startup/absoluteUrl.ts,client/views/root/hooks/useCorsSSLConfig.ts,client/meteor/overrides/oauthRedirectUri.ts— each only exists to bridge Meteor-specific behaviour and has no life after the webapp/DDP removal. Annotated with TODOs.Behavioural notes
Livechat_guest_pool_max_number_incoming_livechats_displayedchanges (previously broken due to.peekinsideTracker.autorun)./tableflip,/lenny,/gimme,/shrug,/unflip) no longer get an optimistic insert since they were only covered by the removedsendMessagestub; the main composer path still does.Scope / out of scope
Still on Meteor and intentionally left for follow-ups:
client/meteor/login/*,UserProvider,AuthenticationProvider,2fa/*,CustomOAuth,loggedIn, etc.SDKClient,RestApiClient,CachedStore,Presence,e2ee/rocketchat.e2e.ddpOverREST,settings,totpOnCall,userAndUsers,unstoreLoginToken.client/meteor/minimongo/*.watch.ts/createReactiveSubscriptionFactory: still Tracker-based; keeping them until their consumers (hasPermission, etc.) are migrated — refactoring the factory now would silently break reactivity for everyuseReactiveValuecall.Test plan
jest app/ui/client/lib/recorderjs/videoRecorder.spec.ts— 8/8.+:emoji:shortcut.Livechat_guest_pool_max_number_incoming_livechats_displayedchanges.