NASA-AMMOS / aerie-ui

The client application for Aerie.
https://nasa-ammos.github.io/aerie-docs/
MIT License
29 stars 5 forks source link

double subscribing on certain hasura queries #1464

Open sqlAlchemyNoob2004 opened 5 days ago

sqlAlchemyNoob2004 commented 5 days ago

Checked for duplicates

Yes - I've already checked

Is this a regression?

No - This is a new bug

Version

2.11.12

Describe the bug

It appears that we are issuing the same hasura subscriptions multiple times on certain endpoints.

Inspecting websocket traffic for subscription SubActivityDirectives I can see that on page load we are sending the same subscribe message twice:

Screenshot 2024-09-16 at 4 01 11 PM Screenshot 2024-09-16 at 4 01 24 PM

Unless I'm missing something, these are two unique subscriptions to the same query with the same planId variable.

I can see future updates over the socket come in twice:

Screenshot 2024-09-16 at 4 02 23 PM Screenshot 2024-09-16 at 4 02 35 PM

Looking at effects.ts it appears we import activityDirectivesDB which is one subscription but we also create another when we call getActivitiesForPlan (https://github.com/NASA-AMMOS/aerie-ui/blob/3f19529b359fbda7af7ec9f6868192554a08a7a2/src/utilities/effects.ts#L2896-L2897).

Does this create a second? Should getActivitiesForPlan just use the activityDirectivesDB which has already been initialized?

I'm not super familiar with aerie so maybe this is by design, but it seems kinda wasteful. Could also be causing unneeded re-renders on data that hasn't actually changed.

Reproduction

just inspect traffic in devtools

Logs

No response

System Info

chrome

Severity

Minor

duranb commented 5 days ago

@sqlAlchemyNoob2004 Yeah, getActivitiesForPlan is more for getting the activities for the plan more immediately for processing plan export, whereas the subscription is to handle updates after loading the plan view. They definitely use the same query except one should just be a fetch vs a websocket. This is still good to look more in to in general though. Thank you!

sqlAlchemyNoob2004 commented 2 days ago

@duranb wanted to update this and say this issue isn't limited to subscriptions

i'm seeing this for regular queries as well, most egregiously on the parsed dictionaries. the response of these queries can be pretty big, so it would be good to not double request them if not needed:

Screenshot 2024-09-19 at 11 42 58 AM Screenshot 2024-09-19 at 11 43 10 AM

duranb commented 2 days ago

@sqlAlchemyNoob2004 nice find! My suspicion is it has something to do with https://github.com/NASA-AMMOS/aerie-ui/blob/develop/src/components/sequencing/SequenceEditor.svelte#L126. Might need to make more granular reactive statements to not capture unnecessary updates to svelte states.