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

Refactor #5025: Add env. vars. for graphql playground and introspection #5055

Conversation

lehnenb
Copy link
Contributor

@lehnenb lehnenb commented Mar 19, 2019

Resolves #5025
Impact: minor
Type: refactor

Issue

In development, Apollo Server enables GraphQL Playground on the same URL as the GraphQL server itself (e.g. http://localhost:4000/graphql) and automatically serves the GUI to web browsers. When NODE_ENV is set to production, GraphQL Playground (as well as introspection) is disabled as a production best-practice.

Solution

Set environment variables with "envalid" in order to determine whether GraphQL Playground should be enabled or not in any environment.

Breaking changes

These changes are all pretty harmless, the only exception to that which has the capacity of causing an error is adding invalid boolean values to the newly added environment vars. (GRAPHQL_INTROSPECTION_ENABLED and GRAPHQL_PLAYGROUND_ENABLED).

FYI - Following are the accepted values for boolean environment variables in the "envalid" lib. : "0", "1", "true", "false", "t", "f"

Testing

  1. Set the GRAPHQL_INTROSPECTION_ENABLED and GRAPHQL_PLAYGROUND_ENABLED;
  2. Access GraphQL Playgrand on the same URL as the GraphQL server.

@lehnenb lehnenb force-pushed the refactor-5025-add-env-for-playground-and-introspection branch 2 times, most recently from 5ae0a09 to 4c605be Compare March 19, 2019 19:40
Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! There are a couple things to fix, but after that should be good to merge.

imports/node-app/core/createApolloServer.js Outdated Show resolved Hide resolved
imports/node-app/core/createApolloServer.js Outdated Show resolved Hide resolved
.env.example Outdated Show resolved Hide resolved
@lehnenb lehnenb force-pushed the refactor-5025-add-env-for-playground-and-introspection branch from 4c605be to 666b347 Compare March 19, 2019 22:25
@lehnenb lehnenb force-pushed the refactor-5025-add-env-for-playground-and-introspection branch from 666b347 to c0e1968 Compare March 19, 2019 22:26
@lehnenb
Copy link
Contributor Author

lehnenb commented Mar 19, 2019

@aldeed, thanks a million for reviewing my PR and for letting me contribute.
Please let me know if there's anything else I can help you guys with 👍

Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. Works!

@aldeed
Copy link
Contributor

aldeed commented Mar 20, 2019

This is ready to merge, but I need to figure out how we can update the integration tests to continue passing. Envalid does not like the value of NODE_ENV when tests are running it seems.

@lehnenb
Copy link
Contributor Author

lehnenb commented Mar 20, 2019

That's Great @aldeed! Also, if you want, I can help you out with those failing tests too, just let me know 👍

Fixes integration test errors

Signed-off-by: Eric Dobbertin <[email protected]>
@aldeed aldeed merged commit 437f2e0 into reactioncommerce:develop Mar 20, 2019
@jeffcorpuz jeffcorpuz mentioned this pull request Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants