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

feat(gatsby): use graphiql-explorer #14280

Merged
merged 25 commits into from May 27, 2019

Conversation

Projects
None yet
3 participants
@pieh
Copy link
Contributor

commented May 23, 2019

Replace current bare graphiql with graphiql-explorer + graphiql

Kapture 2019-05-23 at 21 50 03

Additional benefit that is not really related to graphiql-explorer, but implemented here:
new graphiql integration doesn't use external resources (currently used express-graphql uses https://cdn.jsdelivr.net to get react, react-dom, graphiql etc to render app correctly. This will use local bundle, so you can use it no problem on planes ;)

TO-DO:

  • Write proper README
  • Discuss new package name (?)
  • Consider crafting nice default query - for the most part empty query filled with comments that will target Gatsby users (and maybe graphiql-explorer)
  • Add some e2e tests to to increase confidence in this (and any future changes to this)

Closes #11602

@pieh pieh added the status: WIP label May 23, 2019

@pieh pieh requested a review from gatsbyjs/core as a code owner May 23, 2019

@@ -0,0 +1,53 @@
{
"name": "gatsby-graphiql-explorer",

This comment has been minimized.

Copy link
@pieh

pieh May 23, 2019

Author Contributor

this probably need better package name - this is kind of express middleware, but not exactly, because it's not used as express.use(someMiddleware()), instead it is used as graphiqlExplorer(expressApp, params) as I need to have 2 route handlers (one for html and one for bundle)

This comment has been minimized.

Copy link
@DSchau

DSchau May 23, 2019

Contributor

It's easy to bike shed here--I don't really have a problem with the name. We could also scope it, e.g. @gatsbyjs/graphiql which is really what we've done here. We tweaked graphiql with some extra functionality.

@DSchau

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

@pieh I wrote a README for you, but can't push to the branch! Could you give collaborators (aka me) access?

@DSchau
Copy link
Contributor

left a comment

Looks good to me, left some comments.

In general, I don't necessarily think that:

  • Name change should block, nor
  • Default GraphiQL query should block

I think this is useful enough to ship. Great work!

@@ -0,0 +1,16 @@
# gatsby-graphiql-explorer

This comment has been minimized.

Copy link
@DSchau

DSchau May 23, 2019

Contributor

Simple and concise 🤷‍♂ I think this will suffice!

Show resolved Hide resolved packages/gatsby-graphiql-explorer/package.json Outdated
Show resolved Hide resolved packages/gatsby-graphiql-explorer/src/app/index.ejs
Show resolved Hide resolved packages/gatsby-graphiql-explorer/src/app/webpack.config.js Outdated
Show resolved Hide resolved packages/gatsby-graphiql-explorer/src/app/app.js Outdated

DSchau and others added some commits May 23, 2019

Update packages/gatsby-graphiql-explorer/package.json
Co-Authored-By: Dustin Schau <DSchau@users.noreply.github.com>
Update packages/gatsby-graphiql-explorer/src/app/webpack.config.js
Co-Authored-By: Dustin Schau <DSchau@users.noreply.github.com>
@pieh

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2019

Will try to figure better tests e2e tests tomorrow, not quite sure why failing test is flaking in CircleCi

pieh added some commits May 24, 2019

doh

@pieh pieh removed the status: WIP label May 24, 2019

@DSchau
Copy link
Contributor

left a comment

🚀

@DSchau DSchau dismissed their stale review via 44cbf89 May 24, 2019

pieh and others added some commits May 24, 2019

Update e2e-tests/development-runtime/cypress/integration/functionalit…
…y/graphql-endpoint.js

Co-Authored-By: Dustin Schau <DSchau@users.noreply.github.com>
@DSchau

DSchau approved these changes May 24, 2019

Copy link
Contributor

left a comment

👌 Let's merge!

@pieh pieh merged commit 3863f24 into master May 27, 2019

18 checks passed

Danger All good
Details
Peril All green. Congrats.
Details
ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsbygram Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_pipeline Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_long_term_caching Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: starters_validate Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node10 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node12 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node6 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node8 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_www Your tests passed on CircleCI!
Details
unit_tests_windows Build #20190524.81 succeeded
Details

@pieh pieh deleted the graphiql-explorer branch May 27, 2019

@sgrove sgrove referenced this pull request May 29, 2019

Closed

Integrating with Gatsby? #15

@Kevin-Hamilton

This comment has been minimized.

Copy link

commented May 30, 2019

@pieh - question, how does this impact https://www.gatsbyjs.org/docs/using-graphql-playground/ ? Is the GraphQL Playground also updated with this feature? If not, should that be mentioned in the Docs or Blog post?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.