Create `ZQuery` package for managing data in Zustand #1199

jessepinho · 2024-05-29T03:31:05Z

ZQuery is a new package for managing remote data in Zustand.

Navigating this PR

In this PR, I've created the ZQuery package. The bulk of its code is here, and its types are here.

This PR is a bit large because I've also implemented its use in a few places throughout the app to test it:

The sync status:
- Updated the <SyncStatusSection /> component
- Deleted the <Status /> component that was being used to sync status data to Zustand, since this is now handled automatically by hooks
- Created a status fetcher
- Simplified the status Zustand slice to use ZQuery.
- Updated <AuctionList /> to use useStatus() rather than getting the status from the store
- Updated <TokenSwapInput /> to use useStatus() rather than getting the status from the store
Unclaimed swaps:
- Removed unclaimed swaps from the swap loader, since they'll now be loaded using ZQuery
- Added a ZQuery object to the unclaimedSwaps Zustand slice
- Consolidated logic related to the unclaimed swaps fetcher
- Updated and simplified the <UnclaimedSwaps /> component
Auctions:
- Simplified the dutchAuction Zustand slice to use ZQuery for the auctionInfos array.
- Extract the fetching logic to a new fetcher
- Update the <AuctionList /> component to use the new useAuctionInfos() hook

Main missing features for future improvements

The biggest one: we don't currently cache based on parameters. Ideally, we should be able to do something like this: const auctionInfos = useAuctionInfos({ cacheKey: 'latestState', fetchArgs: [{queryLatestState: true }] });. Then, the results of calling the auctionInfos fetcher with {queryLatestState:true} would be cached under the latestState cache key. If you just call const auctionInfos = useAuctionInfos() — i.e., without any arguments — the results would be stored under e.g., a _zQueryDefaultCacheKey cache key, and would be returned any time a consumer calls useAuctionInfos() without any arguments.

This is actually a bit complex to implement, though, and this PR is a bit large already, so I figured it's best to ship this as-is and revisit this later if needed.
We're not using a finite state machine yet. That is, it's possible for error and data and loading to all be truthy at the same time. In the interest of limiting this PR to a reasonable scope, I'm pushing that off till later.
We don't truly abort calls when a new request is started. We call an abort controller's abort() method, but all that does is stop iterating over the results. I'm not familiar enough with generator functions to know if that actually stops the server from streaming data, but I'm pretty sure it doesn't? Could be wrong, though. In the future, we'll likely want to pass abort controllers (or at least their signal property) to the fetch callback so that we can abort remote calls properly.

changeset-bot · 2024-05-29T03:31:08Z

🦋 Changeset detected

Latest commit: e7b4a41

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages

Name	Type
@penumbra-zone/zquery	Major
@penumbra-zone/getters	Minor
minifront	Patch
@penumbra-zone/ui	Patch
@penumbra-zone/perspective	Patch
@penumbra-zone/query	Patch
@penumbra-zone/services	Patch
@penumbra-zone/storage	Patch
@penumbra-zone/types	Patch
chrome-extension	Patch
node-status	Patch
@penumbra-zone/services-context	Patch
@penumbra-zone/crypto-web	Patch
@penumbra-zone/wasm	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

VanishMax

Impressive work!

Overall, ZQuery feels like a complicated multi-layer abstraction but the one I could personally get used to. I left some comments in form on the conventional comments. None of them actually prompt you to change anything but we could have a discussions in the comment threads to clarify some points

VanishMax · 2024-06-11T10:15:16Z

apps/minifront/src/state/status.ts

+  }>;
+}
+
+export const createStatusSlice = (): SliceCreator<StatusSlice> => () => ({


thought: As a user, I'd expect to use the state inside Zustand with selectors as we always do, so I'd like for ZQuery to just return a slice that I can simply plug into the store and play with it. I don't think the return from ZQuery { status, useStatus } is really relevant. Can we make ZQuery just a kinda create-and-forget type, so all the interactions with it would be through Zustand?

That was actually my initial design for it, but one of the goals of this project was to integrate into the React lifecycle hooks so that data would get fetched on component mount, and any in-flight requests would be aborted on component unmount. Using just selectors means that we would miss out on that benefit.

I do understand the desire for something that "feels" more like Zustand, though. If we decide to merge this PR as-is, we could still revisit that question in the future.

VanishMax · 2024-06-11T10:19:25Z

apps/minifront/src/state/status.ts

+    fullSyncHeight: item.fullSyncHeight,
+    latestKnownBlockHeight: item.latestKnownBlockHeight,
+  }),
+  getUseStore: () => useStore,


question: why do we actually need the whole store for manipulating with just one slice? Aren't getters and setters enough?

Ugh, unfortunately we need it for actually calling the hook (e.g., here). I wanted to avoid this but, since we're actually hooking into the React component lifecycle, we need to actually use the useStore hook (rather than just the getter/setter).

VanishMax · 2024-06-11T10:19:51Z

apps/minifront/src/state/status.ts

+  set: setter => {
+    const newState = setter(useStore.getState().status.status);
+    useStore.setState(state => {
+      state.status.status = newState;
+    });


question: from the DX perspective, can setter accept a newState argument with the getter() return, so that we won't have to query it every time?

Good call. I'd originally used the design you described, then had to switch to this design for some TypeScript reason. I think that reason has since become moot, so I should be able to switch it back. Will look into that and follow up. Thanks for catching that! (This DX is indeed quite confusing.)

OK, wasn't able to resolve this. For other reviewers' benefit, I've messaged Max separately about pairing on resolving the TS issues related to this; until then, though, I'd still love a PR review on this!

grod220

Holy smokes, this is a full blown professional library addon. Think we should pitch Zustand-core for an integration 😅

Think it's definitely in a place where we can feel confident about merging and brainstorming (in a more incremental way) updates, new features, api changes, etc.

Cool stuff!

grod220 · 2024-06-12T14:39:44Z

packages/ui/components/ui/dutch-auction-component/expanded-details/index.tsx

+  const inputExponent = getDisplayDenomExponent.optional()(inputMetadata);
+  const outputExponent = getDisplayDenomExponent.optional()(outputMetadata);


Yep — weirdly, some asset metadata was missing in testnet that I'd had in my wallet.

jessepinho force-pushed the jessepinho/z-query-web-965 branch 11 times, most recently from da2dc89 to 86d9ddb Compare June 6, 2024 05:18

jessepinho changed the title ~~WIP: useZQuery hook for managing data~~ Create ZQuery package for managing data in Zustand Jun 6, 2024

jessepinho changed the title ~~Create ZQuery package for managing data in Zustand~~ WIP: Create ZQuery package for managing data in Zustand Jun 6, 2024

jessepinho force-pushed the jessepinho/z-query-web-965 branch 2 times, most recently from 4531b14 to 35e6633 Compare June 10, 2024 17:45

jessepinho changed the title ~~WIP: Create ZQuery package for managing data in Zustand~~ Create ZQuery package for managing data in Zustand Jun 10, 2024

jessepinho marked this pull request as ready for review June 10, 2024 22:07

jessepinho requested a review from a team June 10, 2024 22:07

VanishMax reviewed Jun 11, 2024

View reviewed changes

VanishMax requested a review from a team June 11, 2024 10:27

jessepinho force-pushed the jessepinho/z-query-web-965 branch from 1d94970 to e619244 Compare June 11, 2024 17:43

grod220 approved these changes Jun 12, 2024

View reviewed changes

jessepinho added 7 commits June 12, 2024 12:53

Create ZQuery lib

7994bc5

Remove unused code

adb0e9f

Tweak syntax and fix performance

6c343cf

Refactor a little

78d3a2f

Change slice name

5e688e1

Fix API to no longer use useStore

559f74d

Simplify a bit

5f8c0b9

jessepinho added 27 commits June 12, 2024 12:54

Fix types again

6569830

Fix auction metadata

fc4f010

Fix QueryLatestStateButton

7c0177c

Remove unused DataTypeInState type

0e1baf8

Require stream to be a function that returns the desired type

813fa0d

Fix type

eecefed

Fix type

3c9bea4

Handle errors in the stream

2a10e24

Fix circular reference issue

1b6b970

Convert status slice to use ZQuery

16f8af5

Simplify type

fb3443b

Fix import

4327715

Fix more type issues; support referencing counting

863540d

Rename var

856be18

Stop streaming results if aborted

21b18b3

Update docs

860dba3

Add comment

9934bdb

Use correct source for latestKnownBlockHeight in TokenSwapInput

e39feb3

Remove unused abortController

f56cc82

Rename fetch types

90c60ae

Remove unneeded named types

1e5ccf0

Remove fetch options for now

3f14e30

Remove unnecessary optional checks

700b33d

Update doc

e4439f1

Make exponents optional

b9a8da0

Remove outdated comment

8c642c3

Add changeset

e7b4a41

jessepinho force-pushed the jessepinho/z-query-web-965 branch from e619244 to e7b4a41 Compare June 12, 2024 19:55

jessepinho merged commit 6b06e04 into main Jun 12, 2024
6 checks passed

jessepinho deleted the jessepinho/z-query-web-965 branch June 12, 2024 20:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create `ZQuery` package for managing data in Zustand #1199

Create `ZQuery` package for managing data in Zustand #1199

jessepinho commented May 29, 2024 •

edited

Loading

changeset-bot bot commented May 29, 2024 •

edited

Loading

VanishMax left a comment

VanishMax Jun 11, 2024

jessepinho Jun 11, 2024

VanishMax Jun 11, 2024

jessepinho Jun 11, 2024

VanishMax Jun 11, 2024

jessepinho Jun 11, 2024

jessepinho Jun 11, 2024

grod220 left a comment

grod220 Jun 12, 2024

jessepinho Jun 12, 2024

		const inputExponent = getDisplayDenomExponent.optional()(inputMetadata);
		const outputExponent = getDisplayDenomExponent.optional()(outputMetadata);

Create ZQuery package for managing data in Zustand #1199

Create ZQuery package for managing data in Zustand #1199

Conversation

jessepinho commented May 29, 2024 • edited Loading

Navigating this PR

Main missing features for future improvements

changeset-bot bot commented May 29, 2024 • edited Loading

🦋 Changeset detected

VanishMax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grod220 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Create `ZQuery` package for managing data in Zustand #1199

Create `ZQuery` package for managing data in Zustand #1199

jessepinho commented May 29, 2024 •

edited

Loading

changeset-bot bot commented May 29, 2024 •

edited

Loading