Skip to content

Add batching to the skybrowser topic#209

Open
ylvaselling wants to merge 2 commits intomasterfrom
feature/skybrowser-topic-batching
Open

Add batching to the skybrowser topic#209
ylvaselling wants to merge 2 commits intomasterfrom
feature/skybrowser-topic-batching

Conversation

@ylvaselling
Copy link
Copy Markdown
Collaborator

Adds the newly introduced batching to the SkyBrowser topic updates

Copy link
Copy Markdown
Contributor

Copilot AI left a 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 integrates the shared Batcher utility into the SkyBrowser topic subscription flow, updating the SkyBrowser reducer to accept partial/batched updates and reduce Redux dispatch frequency.

Changes:

  • Batch incoming skybrowser topic messages via Batcher before dispatching updateSkyBrowser.
  • Update updateSkyBrowser reducer to accept Partial<SkyBrowserUpdate> and apply field-level updates.
  • Adjust derived SkyBrowser state updates to be conditional on receiving browsers data.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/redux/skybrowser/skybrowserSlice.ts Switch updateSkyBrowser to partial updates and conditionally update state fields / derived arrays.
src/redux/skybrowser/skybrowserMiddleware.ts Add Batcher to batch topic iterator updates before dispatching to Redux.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to 53
if (action.payload.selectedBrowserId !== undefined) {
state.selectedBrowserId = action.payload.selectedBrowserId;
}

if (action.payload.browsers && state.selectedBrowserId in action.payload.browsers) {
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

updateSkyBrowser now accepts Partial<SkyBrowserUpdate>, but the reducer still resets the entire SkyBrowser state whenever payload.browsers is missing (because the if (action.payload.browsers && ...) falls through to the reset else). With batching/partial updates this can cause the UI to clear on updates that only include selectedBrowserId/cameraInSolarSystem. Adjust the logic so that the reset/validation only happens when payload.browsers is present, and when payload.selectedBrowserId is present without browsers validate it against the existing state.browsers (reset only if the id is invalid).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@engbergandreas engbergandreas left a comment

Choose a reason for hiding this comment

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

Lol realised now that I'm basically reiterating what Copilot already found 🤷‍♂️

Comment on lines -43 to +50
event: 'stop_supscription'
event: 'stop_subscription'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good catch

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants