graphql-zeus
graphql-zeus copied to clipboard
Types not checked. Apollo typed query hook with Hasura endpoint allows anything.
Is this expected behaviour?
Autocomplete works but the query essentially allows any object without error:

Love Zeus. ❤️⚡️ Thanks.
I think we can repair it thanks!!
Discovered this issue yesterday while testing zeus. It seems like it is only possible to add non-existing fields if there are at least 1 existing field declared in the query.
So as an example:
=== Schema ===
type MalSeasonIdentifier {
season: MalYearSeason!
year: Int!
}
type Query {
animeSeasonList: [MalSeasonIdentifier!]!
}
=== TS === this does not give any errors as 'year' is a valid field ->
const result = await chain("query")({
animeSeasonList: {
year: true,
fieldThatDoesNotExist: "f"
}
})
this does give error ->
const result = await chain("query")({
animeSeasonList: {
fieldThatDoesNotExist: "f"
}
})
Having a safety mechanism to alert us of incorrect queries is important, especially as system evolves and fields may get deprecated and disappear.
Keep up the good work, i hope someone will be able to look into this soon.
Any ideas how this may be fixed? Pretty fundamental for error-avoiding
play with types and try to find a solution. Maybe with latest version of TS you can do smth, before it was impossible for me to do
Also wondering if it's possible to fix this ?
@karibertils https://zeus.graphqleditor.com/page/plugins/typedDocumentNode.html we are in the process of transitioning to this. Try using this way!
@aexol
This looks like a nice way to do things, and I see the types are now working for the variables which is really cool. But typescript still shows no error when adding random fields which don't exist in the schema. See below.
Yes, I still see this problem locally. Pretty significant problem if the whole point is to avoid writing invalid queries. I see this without using hooks -- just the raw client:
const client = (() => {
const chain = Chain('...');
return {
query: chain('query'),
mutation: chain('mutation')
}
})();
const test = async () => {
// Typescript is also happy with this destructuring
const { lots: [{ someFakeField }] } = await client.query({
lots: [
{},
{
id: true,
// this field doesn't exist but doesn't trigger error since 'id' does exist
someFakeField: true,
}
]
});
}
I will say that it seems to handle this well (i.e. error appropriately) when I write it instead as:
const fields: ValueTypes["Lot"] = {
id: true,
someFakeField: true,
};
const { lots: [{someFakeField}] } = await client.query({
lots: [
{
where: {
category: { equals: Category.STEEL }
}
},
fields
]
});
which seems to indicate the type is not narrow enough
I dug a bit into the generated code and noticed this section:
export const Thunder =
(fn: FetchFunction) =>
<O extends keyof typeof Ops, SCLR extends ScalarDefinition, R extends keyof ValueTypes = GenericOperation<O>>(
operation: O,
graphqlOptions?: ThunderGraphQLOptions<SCLR>,
) =>
<Z extends ValueTypes[R]>(o: Z | ValueTypes[R], ops?: OperationOptions & { variables?: Record<string, unknown> }) =>
fn(
Zeus(operation, o, {
operationOptions: ops,
scalars: graphqlOptions?.scalars,
}),
ops?.variables,
).then((data) => {
if (graphqlOptions?.scalars) {
return decodeScalarsInResponse({
response: data,
initialOp: operation,
initialZeusQuery: o as VType,
returns: ReturnTypes,
scalars: graphqlOptions.scalars,
ops: Ops,
});
}
return data;
}) as Promise<InputType<GraphQLTypes[R], Z, SCLR>>;
It seemed to widen the acceptance input type a bit much... I could be wrong, but when I made the change to remove Z
and simply continue using ValueTypes[R]
, it seemed to work appropriately on input. However, removing 'Z' or otherwise typing the fields as ValueTypes["Lot"]
made the response type wider than it should be.
export const Thunder =
(fn: FetchFunction) =>
<O extends keyof typeof Ops, SCLR extends ScalarDefinition, R extends keyof ValueTypes = GenericOperation<O>>(
operation: O,
graphqlOptions?: ThunderGraphQLOptions<SCLR>,
) =>
(o: ValueTypes[R], ops?: OperationOptions & { variables?: Record<string, unknown> }) =>
fn(
Zeus(operation, o, {
operationOptions: ops,
scalars: graphqlOptions?.scalars,
}),
ops?.variables,
).then((data) => {
if (graphqlOptions?.scalars) {
return decodeScalarsInResponse({
response: data,
initialOp: operation,
initialZeusQuery: o as VType,
returns: ReturnTypes,
scalars: graphqlOptions.scalars,
ops: Ops,
});
}
return data;
}) as Promise<InputType<GraphQLTypes[R], ValueTypes[R], SCLR>>;
Any updates on this? If mattkindy's proposed changes works well then maybe look into making a pull request. If not then would be nice if this could be prioritized. As people have noted above, this is currently a big issue and the only blocker for my team to use it.
I will definitely be looking at this again. Have been slammed with other things -- am interested in helping resolving this.
This is a TS problem I will take a look soon. With latest versions of TS it should be possible to resolve it
@aexol @mattkindy Did any of you get further on this? My team needs to migrate soon, would be really great if this could get some love :)
In case it helps someone or the maintainers of this project, we fixed all the typings issues we had with zeus in our fork. Here are 2 PRs that could be useful to someone else:
- https://github.com/elba-security/graphql-zeus/pull/7
- https://github.com/elba-security/graphql-zeus/pull/8
Disclaimer: this is using graphql-zeus v4 as we never upgraded to v5. But I think that some type fixes could be applied to v5 as well.
That looks interesting, in case i want to try that fork then is it possible to get that from a package manager or do i need to compile it myself?
Yes it's available on npm: @elba-security/codegen
See the other commits for small changes as there is no change log. Most changes are bug fixes, others are small adaptations to better fit with Hasura types (uuid or json for example).
@aexol @mattkindy Did any of you get further on this? My team needs to migrate soon, would be really great if this could get some love :)
Afraid not. After @aexol dropped their comment, it coincided with this taking a backseat for us.
@aexol Have you looked further into this issue?
Is this fix on 5.3.2 ? Cause i am testing it and it still seems to accept anything.