Skip to content
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

#693

Closed
toxeus opened this issue Jul 8, 2020 · 3 comments · Fixed by #811
Closed

#693

toxeus opened this issue Jul 8, 2020 · 3 comments · Fixed by #811
Labels
enhancement Improvement of existing features or bugfix k::ui/ux Related to UI/UX experience
Milestone

Comments

@toxeus
Copy link
Contributor

toxeus commented Jul 8, 2020

No description provided.

@toxeus toxeus added bug Something isn't working needs-triage labels Jul 8, 2020
@LegNeato LegNeato added easy enhancement Improvement of existing features or bugfix good-first-issue help wanted and removed bug Something isn't working needs-triage labels Jul 16, 2020
@LegNeato
Copy link
Member

Great idea!

@toxeus
Copy link
Contributor Author

toxeus commented Sep 1, 2022

I was able to resolve this by migrating to async-graphql.

@tyranron
Copy link
Member

tyranron commented Sep 2, 2022

Leaving this still open, as something that could be improved.

toxeus pushed a commit to 21Analytics/lei that referenced this issue Jun 12, 2023
juniper has proven to have some rough edges, for example
inscrutable object validation error messages. Unfortunately,
maintenance on such important issues is sluggish, see e.g. [1].

Therefore, this commit migrates the codebase to use the
async-graphql dependency. Due to the schema aggregation under the
autod umbrella, this needs to be done in one go.

Some of the most important differences between juniper and
async-graphql and their impact on the diff are:

 - async-graphql allows using the same struct as both
   input and output type, disambiguated in the schema by
   `name` and `input_name` directives to async-graphql
   declarative macros

 - async-graphql does not allow coercing multiple congruent
   structs into the same graphql type [2]. This introduces
   types like `OfflineAddress`, `EmailAddress` into the
   schema where before a single `Address` could be used

 - async-graphql makes better use of existing `serde`
   (de-)serialization implementations, through the
   `scalar!` macro

 - async-graphql currently has a bug [3] where two operations
   in a multi-operation document cannot use the same names for
   parameters of different types, leading to some necessary
   renames in integration test `queries.graphql` files

We take the opportunity to replace the `graphql-ws-server` glue
layer with usage of axum's websocket support, conveniently exposed
through async-graphql's axum integration. The exposed routes now
need to match the ones coming from either Caddy / Nginx. Before,
the path was completely ignored by the glue layer. Furthermore,
the axum implementation requires [4] the `sec-websocket-protocol`
header, which we now set in the integration test client. The
server implementation will also set the header on the response,
as was done by hand in the glue layer implementation. This ensures
no regression on a Chrome-specific issue [5].

Finally, a nice side-effect for the developer is that async-graphql
ships the recent graphiql 2, whereas juniper had the ancient 0.17.5.

[1] graphql-rust/juniper#693

[2] async-graphql/async-graphql#1015

[3] async-graphql/async-graphql#1014

[4] https://github.com/async-graphql/async-graphql/blob/70049ae798ce015db9a41b637142af990bffaaf0/integrations/axum/src/subscription.rs#L33-L48

[5] https://gitlab.com/21analytics/21-travel-web-ui/-/issues/397
@toxeus toxeus closed this as completed Oct 25, 2023
@tyranron tyranron reopened this Oct 26, 2023
@toxeus toxeus changed the title Improve input object validation error messages Oct 27, 2023
@tyranron tyranron added this to the 0.16.0 milestone Nov 30, 2023
@tyranron tyranron added the k::ui/ux Related to UI/UX experience label Nov 30, 2023
tyranron added a commit that referenced this issue Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::ui/ux Related to UI/UX experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants