-
-
Notifications
You must be signed in to change notification settings - Fork 79
Refactor: WebSocket from flush queue to one patch #1595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
3de469b
to
c64f242
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff, I left some comments to hopefully improve this logic
Meanwhile, as I was reviewing this, I also found a potential bug.
export function patchRuntimeProperty<K extends keyof RuntimeStore>(key: K, value: RuntimeStore[K]) {
const state = runtimeStore.getState();
state[key] = value;
runtimeStore.setState({ ...state });
}
in this code, we mutate the state before creating a new object with the spread. I believe the correct logic should be
const state = runtimeStore.getState();
runtimeStore.setState({ ...state, [key]: value });
4519bc4
to
997223b
Compare
d24342d
to
051e087
Compare
196e153
to
99a5dbd
Compare
99a5dbd
to
bf4cae5
Compare
bf4cae5
to
59b7ca7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the WebSocket update mechanism by replacing the flush queue with a single patch update approach. Key changes include:
- Removing the previous batching mechanism in EventStore and replacing it with an immediate patch send.
- Updating RuntimeService to use the new batch.add and batch.send APIs.
- Updating the client-side socket handler to process the new "ontime-patch" WebSocket message and remove outdated message types.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
apps/server/src/stores/EventStore.ts | Refactored event store to remove flush queue logic and added a createBatch function that sends a patch update. |
apps/server/src/services/runtime-service/RuntimeService.ts | Modified to aggregate updates in a batch instead of sending multiple separate events. |
apps/client/src/common/utils/socket.ts | Updated to handle the "ontime-patch" message type and removed handling for individual update messages. |
apps/client/src/common/stores/runtime.ts | Removed legacy batch update functions no longer needed due to the new patch mechanism. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this logic, we assume that we we change top level properties of state, and that the top level objects must be complete (no deep patching)
I think that is OK but we need to keep this in mind
* change flush to one patch * remove unused types * create batch * merge patch into eventStore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* change flush to one patch * remove unused types * create batch * merge patch into eventStore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* change flush to one patch * remove unused types * create batch * merge patch into eventStore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* change flush to one patch * remove unused types * create batch * merge patch into eventStore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* change flush to one patch * remove unused types * create batch * merge patch into eventStore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* change flush to one patch * remove unused types * create batch * merge patch into eventStore Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Revert "refactor: add version to session endpoint" This reverts commit cd40a6e. * Revert "Remove public event feature (#1645)" This reverts commit 08d9e24. * Revert "Fix: rearrange playing event (#1640)" This reverts commit 0649678. * Revert "refactor: style tweaks to edit css modal" This reverts commit a00ec2d. * Revert "refactor: remove usages of framer-motion" This reverts commit 54a74cc. * Revert "Upgrade expressjs (#1633)" This reverts commit 6f3ab27. * Revert "Refactor: require trigger in all events objects (#1636)" This reverts commit 90870ec. * Revert "Fix: Correct boundary condition in applyDelay" This reverts commit b640e0e. * Revert "let vite be the proxy to the dev server (#1630)" This reverts commit c41fe82. * Revert "refactor: migrate custom fields to transactions" This reverts commit 62c8319. * Revert "refactor: simplify validations" This reverts commit b1d2346. * Revert "refactor: create transaction system and apply to adding entry (#1620)" This reverts commit 9a62daf. * Revert "Refactor: better rounding (#1594)" This reverts commit b9ab1c6. * Revert "refactor: improve reorder logic" This reverts commit c105471. * Revert "chore: simplify URLs" This reverts commit a4d4f29. * Revert "refactor: order is single source of truth" This reverts commit 2793aad. * Revert "refactor: refetch targets is enum" This reverts commit e7cfb7d. * Revert "refactor: small ux improvements" This reverts commit 256a851. * Revert "refactor: remove trivially inferred numEvents" This reverts commit 4ab9c81. * Revert "feat: duplicate groups" This reverts commit 2f13d6c. * Revert "feat: create group from entry selection" This reverts commit f1f7bad. * Revert "feat: create block from rundown empty" This reverts commit a8b52a4. * Revert "fix: collapsed blocks dont render children" This reverts commit cd0999b. * Revert "refactor: type cleanup and test improvements" This reverts commit b4c60f3. * Revert "feat: allow dissolving a block" This reverts commit 5cefad3. * Revert "fix: uncontrolled prop on controlled component" This reverts commit 7a6ecd8. * Revert "refactor: improve return of reorder" This reverts commit ba96ecf. * Revert "refactor: mutations on batch elements must have IDs" This reverts commit 7bed375. * Revert "chore: upgrade dependencies" This reverts commit e2e755b. * Revert "refactor: extract utility to merge two arrays" This reverts commit a77d231. * Revert "refactor: change network mode defaults" This reverts commit 0eb3b8d. * Revert "fix: delete nested events" This reverts commit 0021185. * Revert "fix: add event at end of block" This reverts commit 94c72ff. * Revert "refactor: make finder available in exported rundown" This reverts commit e4c08dc. * Revert "assert non null and update test (#1604)" This reverts commit ec74af0. * Revert "Fix project renumber (#1597)" This reverts commit b6d72dd. * Revert "Refactor: WebSocket from flush queue to one patch (#1595)" This reverts commit 31c311d. * Revert "Refactor: ms for api calls (#1593)" This reverts commit e9b3cc6. * Revert "fix test (#1601)" This reverts commit 543b04a. * Revert "fix: rebase master" This reverts commit d39b85b. * Revert "chore: correct test path" This reverts commit bbe107b. * Revert "refactor: extract rundown parsing" This reverts commit c616240. * Revert "chore: improve convention entry <> event" This reverts commit 166be66. * Revert "refactor: maintain flat orders" This reverts commit 4180d0a. * Revert "refactor: implement operations on nested events" This reverts commit 78108e3. * Revert "refactor: process events in rundown" This reverts commit 3bb8b70. * Revert "chore: improve convention entry <> event" This reverts commit 1c4f13a. * Revert "chore: rename currentBlock > parent" This reverts commit 3ca0aba. * Revert "refactor: fix delay positioning in gaps" This reverts commit 030c8f8. * Revert "refactor(e2e): skip flaky test" This reverts commit 68175cf. * Revert "refactor: improve project loading" This reverts commit fd8f757. * Revert "refactor: gather group metadata" This reverts commit 876d111. * Revert "refactor: swap maintains schedule" This reverts commit 730cb95. * Revert "chore: rename files" This reverts commit 3c388d4. * Revert "refactor: restructure model to contain an object of rundowns" This reverts commit 2e23718. * Revert "refactor: clearer relationship on rundown elements" This reverts commit 89ea8c4. * Revert "refactor: use strict typing" This reverts commit 178640b. * Revert "refactor: remove stop as a possible end action" This reverts commit 4ed3834. * Revert "refactor: restructure model to contain an object of rundowns" This reverts commit 69eb9a5. * Revert "chore: remove IDE files" This reverts commit 3514251. * Revert "refactor: remove unused and legacy code" This reverts commit 95f2ba3.
No description provided.