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

refactor: use vue-queries for spaces #1191

Open
wants to merge 9 commits into
base: sekhmet/proposals-queries-reactive
Choose a base branch
from

Conversation

Sekhmet
Copy link
Member

@Sekhmet Sekhmet commented Feb 14, 2025

Summary

This PR is resubmit of #1155 (it was reverted after merge) with couple of bug fixes (some issues were fixed in the PR this PR depends on).

Some issues that should be resolved now were caused by the fact that we in some places misused reactivity, which worked with reactive objects (which Spaces used to be), but taking away that implicit reactivity broke it, so we should give this PR proper testing to make sure we are not missing anything.

Depends on #1190

This PR refactors spaces fetching to use vue-query. This removes complex code of spaces composable solving some issues we had before (quickly switching filters for spaces shouldn't break it - only data matching current selection should be visible).

Some things are not handled 100% the way they should (followedSpaces, User profile), because they need refactor of its own. This is already quite big PR so I took some shortcuts there.

Towards https://github.com/snapshot-labs/workflow/issues/410

How to test

  1. Navigate the app (check followed spaces, explore spaces, Settings > spaces).
  2. Navigate into the spaces, everything works as expected, no loading indicators for spaces.
  3. Navigate between different spaces to make sure no stale data is shown (for example clicking through followed spaces).

@Sekhmet Sekhmet force-pushed the sekhmet/spaces-queries branch from d0bb033 to a7e8746 Compare February 14, 2025 17:59
Comment on lines +16 to +19
const spaceIdComposite = computed(
() => `${props.space.network}:${props.space.id}`
);

Copy link
Member Author

Choose a reason for hiding this comment

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

It used to work before because in our case space was reactive object under the hood.

@@ -72,10 +72,10 @@ watchThrottled(
);

watch(
[props.space, state, labels],
([toSpace, toState, toLabels], [fromSpace, fromState, fromLabels]) => {
[() => props.space.id, state, labels],
Copy link
Member Author

Choose a reason for hiding this comment

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

Here it used to work as well, because props.space was reactive, regular props cannot be used as watch source.

@bonustrack
Copy link
Member

Works well, except on the left sidebar with list of following space avatar, this section blink when I come from the home page.
blink

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