-
Notifications
You must be signed in to change notification settings - Fork 126
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
refac: integrate canvas resolver API #6411
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we have two layers of selector functions:
- In the
select
function provided to theResolveCanvas
API's TanStack Query request - The functions provided to the derived stores in this file
Maybe consider if this should be a one-tier system, not two-tier? I wonder if the TanStack Query call and all selectors could all be co-located in this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there's any performance cost of creating a derivable out of query response. The reason I chose to use derivables instead of select
function was the ease of integration in the components. All these selectors derive from a single query so we wouldn't have to check for fetch status or error at every place the selectors are used.
I have added more methods to canvas-spec.ts
in #6396 I have fixed the nits in this PR. We can continue this discussion around selection there.
Integrate canvas resolver API and newly added properties to the YAML.
There are more changes and refactoring needed for time filters. Those will be addressed in a different PR.