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

accept multiple url variants for graphiql IDE #14084

Closed
shannonbux opened this issue May 16, 2019 · 4 comments · Fixed by #14234
Closed

accept multiple url variants for graphiql IDE #14084

shannonbux opened this issue May 16, 2019 · 4 comments · Fixed by #14234
Assignees
Labels
help wanted Issue with a clear description that the community can help with. topic: cli Related to the Gatsby CLI

Comments

@shannonbux
Copy link
Contributor

Summary

In usability tests, we discovered that when people type the wrong thing in the browser (e.g. “localhost:8000/_ _ graphiql), we could either redirect to "correct" one or just support multiple different cases. This will help them spend less time looking for the right link.

Basic example

I'm not sure what the best practice is here. Here are some ideas:

Support various number of underscores before graphiql e.g. _graphiql (1 underscore), __graphiql (2 underscores), ___graphiql (3 underscores), ____graphiql (4 underscores)
Support "graphql" and "graphiql"
Other ideas?

Motivation

We observed that many users (including ourselves!) say "how many underscores is this again? I can never remember" and "I always misspell Graphiql. How do you even say Graphiql??" haha.

@shannonbux shannonbux added the topic: cli Related to the Gatsby CLI label May 16, 2019
@pieh
Copy link
Contributor

pieh commented May 16, 2019

Some tips on how to implement this:
We need to update

if (process.env.GATSBY_GRAPHQL_IDE === `playground`) {
app.get(
`/___graphql`,
graphqlPlayground({
endpoint: `/___graphql`,
}),
() => {}
)
}
app.use(
`/___graphql`,
graphqlHTTP(() => {
const schema = store.getState().schema
return {
schema,
graphiql:
process.env.GATSBY_GRAPHQL_IDE === `playground` ? false : true,
context: withResolverContext({}, schema),
formatError(err) {
return {
...formatError(err),
stack: err.stack ? err.stack.split(`\n`) : [],
}
},
}
})
)

to handle variants mentioned by Shannon in basic example.

To do this we can update paths that our endpoint handle. Express server allow to use path patterns or regular expressions ( https://expressjs.com/en/api.html#path-examples ) and I think those will work here very nicely.

It would be very nice (but not strictly needed) if there were also cypress tests for this in https://github.com/gatsbyjs/gatsby/tree/master/e2e-tests/development-runtime/cypress/integration to make sure that visiting those paths result in viewing graphiql interface

@pieh pieh added the help wanted Issue with a clear description that the community can help with. label May 16, 2019
@thomascorthouts
Copy link
Contributor

@pieh Can I pick this one?

@thomascorthouts
Copy link
Contributor

thomascorthouts commented May 21, 2019

@pieh I have a small question regarding code structuring. I changed the endpoints so they use the express patterns to match everything with at least 1 underscore and match both on graphql and graphiql. So the pattern you get is /_+graphi{0,1}ql

But now I would be interested in creating cypress tests for this as well, where I test whether the patterns give the graphqlIDE, but I don't really see where the best spot would be to put these tests. Should this be a file like cypress/integration/functionality/graphql-endpoint.js or what would be the best place to put this?

@pieh
Copy link
Contributor

pieh commented May 22, 2019

Ah, sorry I didn't reply, but I see you managed. The place for e2e tests is not clear, but one you picked seems very reasonable here

@pieh pieh changed the title Redirect and/or accept multiple options for localhost addresses accept multiple url variants for graphiql IDE May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue with a clear description that the community can help with. topic: cli Related to the Gatsby CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants