-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
Rethink non-Route parameter Variable API #1211
Comments
Yes, exploring new avenues is a good idea. Could this issue also be considered under the About Dynamic ScalarI really like this concept because it's expressive, readable, and aligned with the GraphQL mindset. It might be a bit challenging for newcomers, but perhaps it's our job to make it beginner-friendly 💪. One downside is that you can't directly copy and paste the About Yet-Another-Directive (@variableValue)In my opinion, this doesn't significantly improve the current approach. At present, we instruct users that _variables are all they need to focus on. With this new directive, they'll have to consider a file path, maintain consistency across projects, and manage default exports (or enforce function names or work around string properties). On the flip side, you can point to '/src/variables/getOffsetFormSearchParam' and reuse it across various components. 👍 About Mixed CasesWhat if we have mixed scenarios, like id coming from query ContractOrders($id: ID!, $offset: Int, $info: String) {
contract(id: $id) {
id
name
orders(offset: $offset, info: $info) {
id
quantity
}
}
}
# Using Dynamic Scalar
query ContractOrders($id: ID!, $offset: OffsetFromSearchParam, $info: StringFromSession) {
}
# Using Yet-Another-Directive (@variableValue)
query ContractOrders($id: ID!, $offset: Int, $info: String) @variableValue(fn: "/src/variables/getOffsetFormSearchParam") {
}
# Using Yet-Another-Directive (@checkSearchParam) (@checkSession would look somewhere a global transform?!)
query ContractOrders($id: ID!, $offset: Int, $info: String) @checkSearchParam @checkSession {
} What are the ideal trade-offs? What is most commonly used? I'm eager to hear comments and ideas 😉 |
Looking at all of the options laid out like this, I think the first option is the cleanest. The biggest win in my eyes is the natural ability to compose the different kinds of variables together. The distinction is even more clear if a user is willing to impose a naming convention on these kinds of scalars (another lint rule?) We could pull off something similar if we allowed for # Using Yet-Another-Directive (@variableValue)
query ContractOrders($id: ID!, $offset: Int @variableValue(fn: "/src/variables/offsetFromSearchParam"), $info: String) { Beyond just the noise, there's also some weird coupling here for the generated types since the function has to match the nullability of the field and the same function could be used by multiple fields with different nullabilities.
I think this is the most compelling reason for
Yea! I like this a lot. At the very least we should see if there is a plugin interface we can hook to. that would save us from having to maintain an entire graphiql component for all of the different frameworks.
I think these two issues are distinct and the solution for one might not apply to the other situation. For example, my gut says that driving search parameters with a directive might be a good idea but it doesn't feel like the right fit for non-route parameters. |
One con that occured to me today with the scalar approach is that you can only set one value at a time |
It's a con & pro, being very expressive can be good as well. query ContractOrders($id: Param, $limit: SearchParam, $offset: SearchParam, $info: Session) {} |
That wouldn't quite work - you wouldn't know if you are computing the |
I'm thinking scalars: {
SearchParam: {
graphqlType: "String",
value: (variableName, { url }) => url.searchParam.get("variableName")
}
} I have no clue if/how it's possible yet. But we can probably explore? |
Could the function for resolving get the same inputs as a |
Not sure I understand the concern about type bindings - could you give me an example? My original thinking was that the |
I stumbled upon this issue again after a while, and finally have some time to write out some thoughts. (I assume that this issue is talking about component queries) About dynamic scalarsThese scalars are provided by Houdini and thus don't exist in the schema on the server, making it annoying to work with in external graphql IDEs. In the case of component queries, it would be impossible to pass parameters passed to the component all the way to the client.ts file. I do think that the dynamic scalars could be interesting for more global variables - like the session key provided above - but I don't believe that this could replace the existing magic About another directiveAt first glance this has the same issue with it not existing on the schema. Additionally, having to point to a variable function where the proper value gets passed along sounds very tedious. Maybe this?I do agree that the magic Purely from the perspective of a component query, maybe we can combine the dynamic scalars with a different approach to passing in query params? <script lang="ts">
export let userId: string;
$: store = graphql(`
query MyQuery($id: ID!, $session: StringFromSession!) @load {
user(id: $id, session: $session) {
name
}
}
`, {
variables: {
id: userId
}
// Maybe this space could even be used to pass in other options
// like fetch policy etc
});
</script> This way we don't have to export a black magic What would need some investigating is what would happen if one of the component props gets updated, as that would require a refetch of the component. |
Wow thanks for taking the time to compile your thoughts! I was looking to revamp the entire query variable API but component queries are definitely one of the places the magic function feels the worst. So historically, the reason we have avoided the extra argument to the fwiw, I'm also open to relaxing the need for |
While the magic functions may seem most troublesome for component queries, losing the ability to use Requiring a Could it perhaps be possible to have a |
Assuming i'm following you correctly @tommyminds, I think this is pretty much what we have now. Users have to define a function in |
I think the thinking for streamlining should probably start at where other than routes a user might get a variable from:
Maybe there can be some directive to "fill" a variable from various sources? This could be a leaky if its resolved clientside so it should be a generated directive, but it could be like |
Funny you should mention that @endigma - i was thinking something very similar last night but wasn't quite sure how to dress up the API. I think the core insight in this is that the inputs of this function are pretty much fixed to things that are accessible on the server. I think the biggest hurdle will be to validate the dot notation, although maybe its just an acceptable compromise that we don't know the shape of your Session (without doing some typescript introspection that seems too hard to get right to be worth it) |
Another thought I had is to restructure the current import { QueryVariables } from './$houdini'
export const _variables: QueryVariables = ({ session } ) => {
return {
OrganizationInfo: {
variable: session.organization
}
}
} As a low-level API this feels slightly better since there's no need to worry about the overlapping type and function name and its more clear that you can pass values for each query available to your route. This comes at the cost of not being able to catch that they are defining functions for every query that needs it. For example, if there are multiple queries that need variables, we would only know they are passing values for a specific query by analyzing the return structure of the variable function and that can be pretty brittle - users like to do crazy things in functions when they can. |
I'm going to close this since RuntimeScalars are now a thing. will come back to this later |
Describe the feature
Right now, if a user wants to provide data for a query that defined inside of
+page.gql
then they currently have 2 options:_{QueryName}Variables
function in+page.js(x)
that returns the value.The unfortunate reality is that not all parameters can be naturally encoded in the URL. Take for example Vercel's profile switcher. Since the value for which org is selected is not encoded in the URL, the use has to split their data-fetching logic across 2 files:
+page.gql
+page.js(x)
This feels very disconnected. I'd like to see if we can come up with a solution that feels more streamlined with this approach being as last-ditch escape hatch to wire up query values.
Obviously the biggest constraint in the API is how to clearly map query variables to a javascript function. The current solution does this by looking for a magical function. I have a few ideas for ways to pull this off
DynamicRuntime ScalarOne idea I had was to define something like a custom scalar (ie,
OrganizationFromSession
that you can configure to be equivalent to aString
but with a value that's computed at runtime:we already enforce that the config file needs to be importable by the server and client so i think we can pretty safely leave it up to the user to make sure the body of that function always works (ie, it can't just always read from local storage)
This API works really well for values that are core to an application-specifics. It probably only feels good for values that are global (ie in the session).
pros:
cons:
Yet-Another-Directive
I think this one is slightly more beginner friendly:
Pros:
Cons:
Criticality
cool improvement, my projects will benefit from it
cc @pixelmund @jycouet
The text was updated successfully, but these errors were encountered: