Open Martinsos opened 2 years ago
Hey @Martinsos nice job! Enjoyed reading and thinking about all the possibilities :) The motivation and requirements really got me aligned to the needs here. As for the code examples, this area is newish to me, so take all the following with a grain of salt, but had some questions to further help me cement the ideas.
getTasks
to receive { taskTag }
as args? The name?getUser,
getAnotherUser: getUser,
what if instead we had some way to say this query is registered to run over this list of arguments? I'm just wondering what if we have N similar things to fetch, like all the groups a user belongs to, for example.
getUser,
getAnotherUser: getUser,
getTasks: (getTasks, {
getArgs: import { getTasksArgs } from "@ext/components/MyComponent",
dependsOn: ["getUser"]
})
Does this really translate to:
getUser
depends on nothing, and gets no args, and maps to itself?getAnotherUser
is the same as getUser
(i.e. maps to), or depends on?dependsOn
be a list of identifiers vs strings, just for symmetry on LHS?Apologies if I missed the boat on some of the code, happy to chat if easier to clear up that way as well. Thanks and nice work! 🥳
The question "Is this similar to React Suspense" was raised and I was unable to answer, so I should investigate "React Suspense" closer to see how they are related.
@sodic suggested that instead of introducing map
we might want to overload dictionaries so they can support this -> that is an interesting proposal.
After debating on it altogether, we concluded that the idea makes sense in general. Still, its value is not yet completely clear -> does it bring enough value to be prioritized over other features?
Therefore, we concluded that we should try collecting more evidence to give us a better idea of its value before taking any further steps.
Some ideas on how to collect it (therefore our TODO):
[README](https://github.com/wasp-lang/wasp/blob/main/examples/realworld/README.md)
:When I would use a couple of useQuery statements in the same React component, I would quickly start feeling like there is too much state to manage. Each of these statements can fail with an error, it could be loading, it could be in its initial state, it could be loaded, it could be fetching but it is keeping previous data (keepPreviousData) -> so each statement results in a bunch of possible states, and if we have a few of them, we end up with a crazy amount of combined states. Not to mention dependent queries and the fact that we can't return early and similar. Writing logic that handles all the cases seems hard and over the top, but on the other hand not handling everything feels lacking. Maybe I just need to put more effort into this and it is really not so bad as it seems? Anyway, it might be interesting investigating if this could somehow be simplified with Wasp, maybe by putting some structure to queries and providing some initial infrastructure for handling all these cases -> in this case we would probably have queries as a part of Wasp directly.
Hey @Martinsos nice job! Enjoyed reading and thinking about all the possibilities :) The motivation and requirements really got me aligned to the needs here. As for the code examples, this area is newish to me, so take all the following with a grain of salt, but had some questions to further help me cement the ideas.
- Do we have ideas how we would handle pagination and other user-action driven inputs that may modify query args?
Hm that is very interesting -> so if they are coming through props, we can pass them to queries. The only thing we might have issues with are other react hooks that are not queries - we don't have a way to depend on them with our queries! I am not sure if we can even do that via pure React hooks though? Can useQuery depend on the useState or useEffect that is above it? It might be able to do so? Very good question!
- In the minimal example, what is the wiring that enables
getTasks
to receive{ taskTag }
as args? The name?
Ah yes I made that very unclear, I now updated the previous comment to actually explain this correctly -> both are passed the same object with props, and only getTasks uses it, assuming it is actually ok with receiving { taskTag }
.
Instead of getting 2 users like so (I assume?):
getUser, getAnotherUser: getUser,
what if instead we had some way to say this query is registered to run over this list of arguments? I'm just wondering what if we have N similar things to fetch, like all the groups a user belongs to, for example.
Ha interesting! I am guessing though in practice you will usually want to have a different query in such situation though, one that fetches multiple users, so maybe this is not a problem?
I am having a bit of a hard time groking how the defaults work in the first two cases vs the explicit third:
getUser, getAnotherUser: getUser, getTasks: (getTasks, { getArgs: import { getTasksArgs } from "@ext/components/MyComponent", dependsOn: ["getUser"] })
Does this really translate to:
getUser
depends on nothing, and gets no args, and maps to itself?getAnotherUser
is the same asgetUser
(i.e. maps to), or depends on?- Could
dependsOn
be a list of identifiers vs strings, just for symmetry on LHS?
Well good points this is very unclear indeed! So all of these are by default passed all of the props from component (which I didn't explain well previously but corrected that now), except for the third one which defines its own function that maps component props to query args. As for names and such: so keys in map are names for the instances of these queries, while identifiers on the right are actual Queries we are referencing. Why do we even need to assign them names? In order to be able to define dependencies between them, if needed. It is a bit confusing though that I use identifier on one place, then string a bit later, hard to say what are names, what are queries, ... . We should probably find some nicer way, if we can.
Apologies if I missed the boat on some of the code, happy to chat if easier to clear up that way as well. Thanks and nice work! partying_face
Nope you got it all right, great questions!
The main concern I have with this feature is it adds a lot of complexity to the DSL. The feature does two things:
(1) does not appear to be actually useful so far: we have no current or planned use case for this knowledge. Perhaps if, in the future, we find some situation where this would be useful, we should look at this feature more.
And for (2), similar ergonomics could be had by giving users a richer library to work with queries in JS code. I will discuss this idea later.
As for the complexity added, the main thing is that this is moving a lot of UI logic to the Wasp file. While it does seem nice to be able to quickly make a bunch of queries available to a page, I have concerns that this would start to be more awkward/cause feature creep when users want to use declarative queries for more complex features. This is because even more logic would have to move into the Wasp file, and we start feeling a need for inline JS in Wasp which, to me, feels like it defeats the declarative nature of Wasp.
Additionally, one place where I'm not sure how this feature would work is with the dependsOn
field. Does this mean a query can access the result of a previous query to use as an argument? How would that look in JS? Using strings to refer to other queries (in dependsOn
) also feels odd, since it seems like it wouldn't be statically checked that the query exists. To fix this, we would need to introduce some sort of variable binding and scope rules to limit reference to the queries to within that component declaration, which is more complexity to add to the DSL.
As mentioned above, a richer library for queries in Wasp could give similar ergonomics to this feature. The basis of this library would be introducing what is essentially a query monad. This won't be that weird or scary for JS developers, since they are already used to the Promise API, which is just a monad. An example of this might be:
import getUser from '@wasp/queries/getUser'
import getCommentsByUser from '@wasp/queries/getCommentsByUser'
import getPostForComment from '@wasp/queries/getPostForComment'
import * as q from '@wasp/queries'
const getPostsWithCommentsByUser = getCommentsByUser.then(comments =>
q.map(comment => getPostForComment.with({ commentId: comment.id }), comments)
)
const UserActivity = (props) => {
return (
<div>
<h1>Posts interacted with by {props.user.name}</h1>
<ul>
{props.posts.map((post, idx) => (
<li key={idx}>{post.title}</li>
))}
</ul>
</div>
)
}
export default q.wrap({
queries: {
user: getUser,
posts: getPostsWithCommentsByUser
},
getArgs: (props) => ({ userId: props.match.param.id }),
component: UserActivity
// loading and error components are not specified, so default components are used
})
The monadic interface for queries is demonstrated mainly in the definition of getPostsWithCommentsByUser
.
q.new
takes a function of one argument that returns a promise (it's argument is given to it by useQuery
)q.all
takes an array of queries and waits for all of the queries to finish<query>.then
runs another query after the first query finishes (arguments are given to the first query when calling useQuery
)q.map(f, arr)
is equivalent to q.all(arr.map(f))
useQuery(<query>.with(args), otherArgs)
is equivalent to useQuery(<query>, args)
, i.e. with
binds arguments to a query before it is runq.wrap
makes the results of all the queries given to it available in the components properties. q.wrap
also passes through the properties it was given to the component it wraps.Note that the actual component code (UserActivity
) does not care at all about the queries, it just receives the results in its properties.
This API is a bit awkward right now, but I think with more thought put behind naming and other decisions, it would be nice to use.
A lot of my motivation for my thoughts on this is from experience I had using Wasp in a project where some components had one query depending on the result of others, which, for me, was the biggest pain points with queries. So making chains of queries feel natural would be my priority with a feature for queries.
This comment ended up being a lot less about the RFC than I expected, but I feel its important to show an alternative, since we certainly need something to help managing multiple queries.
Thanks for this @craigmc08 very interesting!
I agree about the point (1) -> we don't yet have a good idea of how Wasp knowing about query <-> UI relationship is useful, so the value that might come from that is unrealized for now. And I think it is a good idea to wait therefore -> if we figure out in the future the value behind this, it would give us better incentive to pursue this RFC.
I think the language itself could support it -> even if we don't do static analysis at type level for dependsOn
, we can still add a check later during validation of AppSpec
, we already check some of the stuff there -> it is not as elegant as checking stuff on type level, but in practice it works relatively ok.
Good idea on exploring others ways to solve the problem (2)! @matijaSos will be happy that there is at the end indeed a monad in play hehe :D. I didn't yet figure out all the details of your idea but it makes general sense -> basically we solve the same problem of giving structure to multiple queries in JS, now in Wasp, be it via "monad" or some other solution. And our "monadic" library provides a way to compose and combine queries nicely.
Interesting that react-query doesn't have such solution though -> or maybe it does and we missed something? If we are going in this direction it would make sense to rethink stuff from a fresh perspective.
Let's leave this open for now then, until we decide to push more in the JS direction (which might be a bit less priority compared to some other stuff we have going on) or until we get more evidence that having this knowledge in Wasp is useful.
Motivation
Queries are the main way in Wasp to fetch data and use it in view logic (Pages).
They are used as React hooks currently, which puts them somewhere between the imperative and declarative world.
Since dealing with result from queries as hooks in the React component can be complex, especially if there are a couple of them, and since they already have declarative nature, idea is that we could describe Queries used by specific React Component (Wasp Page) directly in Wasp, as declarative statements.
That way Wasp would get more knowledge about which Queries/data does which piece of UI logic (Page) use, and Wasp could also offer some additional machinery around it, to handle common use cases regarding the lifecycle of fetching data.
Sources of inspiration:
To summarize:
Wasp Page vs Wasp Component
While inspiration for this feature came while looking at Wasp Pages, it quickly becomes evident that it could easily be generalized to any React component, not just Pages (which are components at the roots of the UI trees).
And it makes sense to do so -> there doesn't seem to be any good reason to limit its benefits only to the Pages.
This would however require new concept/declaration in Wasp, which is
Component
, so that we can attach the Queries to it.The only reason, it seems to me, that we would implement this for Pages but not for Component, is to make a smaller first step, in transition to adding
Component
declaration and implementing support for declarative Queries for it.Requirements
Basic (required)
Loading
.Failure
.(2), (3), (4) and (5) are required for proper management of query's fetching lifecycle, but we can provide defaults, in case developer doesn't want to specify them.
Advanced (optional)
Redwood Cell is great inspiration for all of this! Redwood Cell uses
beforeQuery
for (2),Loading
for (4),Failure
for (5),isEmpty
andEmpty
for (7),afterQuery
for (8). And,Success
for successfully fetched data.NOTE: It can be completely possible that one Page/Component depends on the same Query multiple times! For example, one Page might need to fetch two users, user A and user B, and will therefore depend on Query
getUser
two times! We need to make sure to be able to support this.Solution ideas
Minimal example (basic requirements)
Per defaults, both queries will receive all component props as args, so in this case they will receive
{ taskTag }
as args, whichgetTasks
can then use, andgetUser
will just ignore it.Loading, error, building query args → all that happens using defaults (which is why component props are just passed to queries as args).
Advanced example (with advanced requirements also)
This example has all the features (
onLoading
,onError
, dependencies, two calls to same query):Here we needed to introduce new concept in language, “map”, in order to enable defining dependencies between queries -> since there can be two same queries, we can't just use query function itself to refer to another query, we need to have same kind of naming, therefore "map".
What is not great is amount of ExtImports → we are repeating ourselves a lot and it looks pretty boilerplatish.
Another way could be to provide JS runtime API for registering "Wasp Component", and then using that to combine together all pieces of it before referencing them from .waps file. Something like this:
Conclusion
This seems like a logical step toward capturing more stuff in Wasp and offering more value to the users. Redwood Cell is a big inspiration for it. However, benefits are not completely clear yet, as well as some implementation details.
Some of the main questions that remain open: