Skip to content
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

fix: Only call store APIs once #3306

Conversation

michael-genson
Copy link
Collaborator

What type of PR is this?

(REQUIRED)

  • bug

What this PR does / why we need it:

(REQUIRED)

Currently our frontend stores (e.g. foods store) shares the store state globally so changes in one component effect another (e.g. if a food is created, it should appear on other components). What's not shared, however, is the loading variable. While in most situations this doesn't matter much, when initializing the store every single component makes an API call to populate the store data (and all of them overwrite each other).

This is most egregious on the recipe editor. If, for instance, you have 20 ingredients, you're making 20 API calls to the food and unit stores and serializing the entire store 20 times. Here is a snapshot of API calls from simply clicking edit once on a single recipe with 20 ingredients (I can't fit the whole thing in a reasonably-sized screenshot):

image

This PR adds a shared loading state so the API is only called once per store. Here is a snapshot of the same recipe with these changes (this time with every API call in the image):

image

Which issue(s) this PR fixes:

(REQUIRED)

N/A

Special notes for your reviewer:

(fill-in or delete this section)

Interestingly, while we did have a "loading" variable already, it wasn't being set correctly (it was set outside of the async function so it toggles immediately back to false) and it wasn't ever being referenced, it was only ever declared and updated. However, even if we fixed all of those things, it still wasn't shared, so each instance of the store had its own loading value.

Testing

(fill-in or delete this section)

Other than the above screenshots, I went and touched all the other stores (e.g. the data management page) and confirmed it works as expected. I can't see any side-effects of a shared loading state since the store always uses the default params (no filters, return all items without pagination). If we used variable params this wouldn't work without some modification.

@hay-kot hay-kot enabled auto-merge (squash) March 12, 2024 22:35
Copy link
Collaborator

@hay-kot hay-kot left a comment

Choose a reason for hiding this comment

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

LGTM

@hay-kot hay-kot merged commit 42523bb into mealie-recipes:mealie-next Mar 12, 2024
10 checks passed
@michael-genson michael-genson deleted the fix/recipe-editor-loads-slowly branch March 12, 2024 22:38
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.

2 participants