fix(graphql_playground): use graphiql instead #2446
Conversation
rymnc
requested review from
xgreenx,
Dentosal and
MitchTurner
as code owners
rymnc
force-pushed
the
test/add-changes-to-graphql
branch
from
7eb4acf
to
d6ed8de
Compare
rymnc
force-pushed
the
test/add-changes-to-graphql
branch
from
d6ed8de
to
cc4f659
Compare
| @@ -31,7 +31,7 @@ pub struct GraphQLArgs { | |||
| pub graphql_max_complexity: usize, | |||
|
|
|||
| /// The max recursive depth of GraphQL queries. | |||
| #[clap(long = "graphql-max-recursive-depth", default_value = "16", env)] | |||
| #[clap(long = "graphql-max-recursive-depth", default_value = "24", env)] | |||
There was a problem hiding this comment.
16 is not sufficient for graphiql, needing this bump
There was a problem hiding this comment.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no suggestions.
|
|
||
| let graphql_playground = | ||
| || render_graphql_playground(graphql_endpoint, graphql_subscription_endpoint); | ||
|
|
||
| let router = Router::new() | ||
| .route("/v1/playground", get(graphql_playground)) |
There was a problem hiding this comment.
We can support old endpoint under v1 and new under v2=)
There was a problem hiding this comment.
What's the point ? I think the new one has all the same features and it's just a playground no code depends on it.
There was a problem hiding this comment.
In this case this change will be non breaking and we don't need to update documentation plus people who used to use the old playground can continue to use it.
There was a problem hiding this comment.
the old playground has security issues too so I'm not sure we should keep it
There was a problem hiding this comment.
For me when you perform an update, you can except some UI changes as soon as it doesn't break any code I don't think anyone is doing scrapping on the graphql playground and I see no point of using the previous ones all features seems to exist on this one too.
There was a problem hiding this comment.
However, I don't want to block anything just "for this" so it's a deal breaker for you @xgreenx I'm ok to walk beside you on that.
Linked Issues/PRs
Description
The graphql-playground is well-outdated, with the last update being 2y ago. This PR switches it out for graphiql which is more recently maintained.
I did this because the tooltips don't dissapear on the playground with latest versions of chrome/arc due to a deprecated event handler on the DOM.
see graphql/graphql-playground#1429
Checklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]