Move `gui` option into constructor and use `playground` #1297

zionts · 2018-07-04T21:58:02Z

Although I know we want to remain less tied to the GraphQL Playground
GUI options, we definitely want to support a wider variety of options to
be passed in. This adds support for specifying partial options either
statically or dynamically for the gui, which can be extended to allow
for a wider array of guis than only GraphQL playground.

TODO:

Update CHANGELOG.md with your change (include reference to issue & this PR)
Make sure all of the significant new logic is covered by tests
Rebase your changes on master so that they can be merged easily
Make sure all tests and linter rules pass

Although I know we want to remain less tied to the GraphQL Playground GUI options, we definitely want to support a wider variety of options to be passed in. This adds support for specifying partial options either statically or dynamically for the gui, which can be extended to allow for a wider array of guis than only GraphQL playground.

evans

@zionts Thank you so much for the PR! A couple of requests to bring the settings more in line with the other patterns we uses in the ApolloServer constructor.

Love the is Partial, definitely learned something with this PR.

evans · 2018-07-05T23:01:19Z

packages/apollo-server-express/src/ApolloServer.ts

+        partialGuiOverrides = gui(req);
+      }
+
+      console.log('partial enabled', partialGuiOverrides.enabled);


Let's remove the console.log's before we merge

Whoops! Thanks for catching.

evans · 2018-07-05T23:03:36Z

packages/apollo-server-express/src/ApolloServer.ts

@@ -29,9 +30,27 @@ export interface ServerRegistration {
  bodyParserConfig?: OptionsJson | boolean;
  onHealthCheck?: (req: express.Request) => Promise<any>;
  disableHealthCheck?: boolean;
-  gui?: boolean;
+  gui?: ((req: express.Request) => Partial<GuiOptions>) | Partial<GuiOptions>;


I think it's important to retain the option to use or return a boolean for the gui option. Also, we'll have to include this logic across all of the integrations that use a gui. If you think of another way to encapsulate this logic, I'm happy to go for it.

evans · 2018-07-05T23:06:19Z

packages/apollo-server-express/src/ApolloServer.ts

 }

+export interface GuiOptions {
+  enabled: boolean;


I don't think we need to include an enabled boolean, provided that we can use a boolean instead of a configuration. subscriptions are a good example of how this configuration is done in the engine constructor.

evans · 2018-07-05T23:08:11Z

packages/apollo-server-express/src/ApolloServer.ts

 }

+export interface GuiOptions {
+  enabled: boolean;
+  playgroundSettings: Partial<ISettings>;


@stubailo was interested in a defaultQuery option. This should probably include a query, variables, operationName, and headers. The playground settings could take precedence with a console.warn

I've added in the GraphQL Tabs options, which support this by default. If we add more options in the future, we may want to have a graphqlPlayground field which specifies all of the settings we allow and do a check that exactly one gui option is configured, but I really don't feel the need to over-abstract something that only has one allowable GUI at the moment.

evans · 2018-07-05T23:20:44Z

It's also important to note that these changes should be added to the Playground documentation https://www.apollographql.com/docs/apollo-server/v2/features/playground.html. Probably in a separate PR

zionts · 2018-07-06T07:02:32Z

@evans I'm happy to add to the docs as well, but I'm waiting on a merge since I don't want to have to re-write them 😝

linonetwo · 2018-07-07T14:54:09Z

Does GUI have a defaultApiUrl option?

For example, https://scaneos.io/gqapi/graphql When opened, it uses https://scaneos.io/graphql as default API URL, while it's wrong.

GraphQL Engine have this option:

engine.listen(
  {
    port: 3002,
    expressApp: app,
    graphqlPaths: [process.env.NODE_ENV === 'production' ? '/gqapi/graphql' : '/graphql'],
  })

zionts · 2018-07-09T01:30:27Z

packages/apollo-server-express/src/ApolloServer.ts

+        ? {}
+        : nonDynamicGui;
+
+      // Disallow endpoints in pre-fill settings


I don't think we actually need to do this. I think this was my misunderstanding what "endpoint" is

You're assertion is correct. The endpoint is where that tab is pointed at

evans

We're almost there! I'm okay with the fact that we are only supporting apollo-server/-express for now. It will make future contributions much easier if the gui configuration functions are placed apollo-server-core.

Thank you again for the PR!

evans · 2018-07-09T06:05:01Z

packages/apollo-server-express/src/ApolloServer.ts

+        ? {}
+        : nonDynamicGui;
+
+      // Disallow endpoints in pre-fill settings


You're assertion is correct. The endpoint is where that tab is pointed at

evans · 2018-07-09T06:08:39Z

packages/apollo-server-express/src/ApolloServer.ts

    app.use(path, (req, res, next) => {
-      if (guiEnabled && req.method === 'GET') {
+      // We cannot do an existence check on `gui` since it may be a boolean
+      const nonDynamicGui: NonDynamicGuiConfig =


Can we put this configuration in a helper function and place the setting of guiOptions after the cors options above?

It will put the configuration code together and reduce the complexity of this middleware

evans · 2018-07-09T06:09:46Z

packages/apollo-server-express/src/ApolloServer.ts

+
+// This is necessary to make TypeScript happy, since it cannot smart-cast types via conditional logic
+function isBoolean(gui: NonDynamicGuiConfig): gui is boolean {
+  return typeof gui === typeof true;


I think this should be possible with an if statement like so:

if(typeof gui === 'boolean') { ... }

Okay! Yay for TS compile-time type-casting not being absolute trash 🎉 I preferred doing it the ... typeof true way to leave out the hard-coded string. I assume that's fine, but you'd prefer this logic not to be made a function if it doesn't need to be.

akand074 · 2018-07-10T00:15:03Z

packages/apollo-server-express/src/ApolloServer.ts

@@ -160,6 +224,8 @@ export class ApolloServer extends ApolloServerBase {
            endpoint: path,


Is it possible to make this endpoint configurable? or should we be using tabs like:

{ tabs: [{ endpoint: '/graphql' }] }

Though it doesn't you can set a subscription endpoint for a tab.

ashtonsix · 2018-07-10T14:19:36Z

@zionts Hey, it looks like this supports Tabs & ISettings but not GraphQLConfigData. GraphQLConfigData has this really awesome feature which lets you group tabs by endpoint & environment; additionally, new tabs within a group use the correct headers and endpoint automatically. It's incredibly useful, but not possible to use in apollo-server@rc.6 because gui passthrough was turned off - if you could add support for grouping tabs to your pull request it'd make me really super happy! 🙏 😄

This is what it looks like:

The number next to each group indicates how many tabs are open. I'm not sure how, but the playground remembers which tabs I have open between server restarts. I'd love to configure default tabs for new team members & to add these "remembered" tabs with a partial (& a restore defaults button!) but that might be out of scope.

Here's how I got this working with apollo-server@rc.5:

.graphqlconfig

{
  "projects": {
    "salamander-apollo": {
      "schemaPath": "schema/schema.gql",
      "extensions": {
        "endpoints": {
          "dev": "http://localhost:6100/graphql",
          "prod": "https://salamander.ai/graphql"
        }
      }
    },
    "salamander-prisma": {
      "schemaPath": "../salamander-prisma/generated.graphql",
      "extensions": {
        "endpoints": {
          "dev": "http://localhost:4466/salamander",
          "prod": {
            "url": "${env:PRISMA_ENDPOINT}",
            "headers": {
              "Authorization": "Bearer ${env:PRISMA_TOKEN}"
            }
          }
        }
      }
    }
  }
}

getGraphQLConfig.js

const treeMap = (v, f) => {
  v = f(v)
  if (typeof v === 'object' && v) {
    Object.keys(v).forEach(k => (v[k] = treeMap(v[k], f)))
  }
  return v
}

const getGraphQLConfig = () => {
  const graphqlConfigPath = path.resolve(__dirname, '../../.graphqlconfig')
  const graphqlConfig = JSON.parse(fs.readFileSync(graphqlConfigPath, 'utf8'))
  treeMap(graphqlConfig, v => {
    if (typeof v !== 'string') return v
    return v.replace(/\${env:([^}]+)}/g, (m, k) => process.env[k])
  })

  return graphqlConfig
}

index.js

server.applyMiddleware({app, gui: {config: getGraphQLConfig()}})

martijnwalraven · 2018-07-11T15:57:02Z

Thinking about this more, I think a cleaner solution is to rename gui to playground and nest the options directly under there. At this point, playgroundOptions is just another level of indirection, but we're not really abstracting over different GUIs.

akand074 · 2018-07-11T16:57:02Z

packages/apollo-server-core/src/playground.ts

+export function createPlaygroundOptions(
+  gui: GuiConfig = {},
+): PlaygroundRenderPageOptions | undefined {
+  const isDev = process.env.NODE_ENV === 'production';


Ah yes! Good catch. Thank you

ashtonsix · 2018-07-11T23:34:40Z

@evans I only have my phone until Monday, so it's a little hard to tell but this supports playground.config, right? Does this mean we can close #1290?

zionts changed the title ~~Add a wider diversity of gui options~~ [WIP] Add a wider diversity of gui options Jul 4, 2018

zionts force-pushed the adam/gui-options branch from 8998970 to 0e329b9 Compare July 4, 2018 21:59

zionts changed the title ~~[WIP] Add a wider diversity of gui options~~ Add a wider diversity of gui options Jul 4, 2018

evans reviewed Jul 5, 2018

View reviewed changes

Add boolean option and configuration for tabs

e54f9be

zionts force-pushed the adam/gui-options branch from eb3b528 to e54f9be Compare July 6, 2018 07:01

zionts commented Jul 9, 2018

View reviewed changes

evans reviewed Jul 9, 2018

View reviewed changes

akand074 reviewed Jul 10, 2018

View reviewed changes

evans mentioned this pull request Jul 10, 2018

Passthrough of options to graphql playground disabled #1290

Closed

evans added 4 commits July 10, 2018 23:33

move gui setting into ApolloServer Constructor

00e4a1e

Merge branch 'version-2' into adam/gui-options

3a6189b

document playground configuration in the constructor

20658d7

update playground types and fixed micro + koa integrations

e767cd4

akand074 reviewed Jul 11, 2018

View reviewed changes

evans added 3 commits July 11, 2018 14:00

change gui to playground

e68ad74

docs: change gui to playground

18b11de

fix logic for playground creation

a32533e

evans changed the title ~~Add a wider diversity of gui options~~ Move gui option into constructor and use playground Jul 11, 2018

evans merged commit 11b8671 into version-2 Jul 11, 2018

zionts deleted the adam/gui-options branch July 12, 2018 00:03

dependencies bot mentioned this pull request Aug 15, 2018

Update apollo-server-express in / from 1.2.0 to 2.0.2 LibreTubeApp/LibreTube#158

Open

danieljuhl mentioned this pull request Oct 10, 2018

"gui" option for server.applyMiddleware does not exist #1717

Closed

github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `gui` option into constructor and use `playground` #1297

Move `gui` option into constructor and use `playground` #1297

zionts commented Jul 4, 2018 •

edited

Loading

evans left a comment

evans Jul 5, 2018

zionts Jul 6, 2018

evans Jul 5, 2018

evans Jul 5, 2018

evans Jul 5, 2018

zionts Jul 6, 2018

evans commented Jul 5, 2018 •

edited

Loading

zionts commented Jul 6, 2018

linonetwo commented Jul 7, 2018 •

edited

Loading

zionts Jul 9, 2018

evans Jul 9, 2018

evans left a comment

evans Jul 9, 2018

evans Jul 9, 2018

evans Jul 9, 2018

zionts Jul 9, 2018 •

edited

Loading

akand074 Jul 10, 2018

ashtonsix commented Jul 10, 2018 •

edited

Loading

martijnwalraven commented Jul 11, 2018

akand074 Jul 11, 2018

evans Jul 11, 2018

ashtonsix commented Jul 11, 2018

		@@ -160,6 +224,8 @@ export class ApolloServer extends ApolloServerBase {
		endpoint: path,

Move gui option into constructor and use playground #1297

Move gui option into constructor and use playground #1297

Conversation

zionts commented Jul 4, 2018 • edited Loading

evans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evans commented Jul 5, 2018 • edited Loading

zionts commented Jul 6, 2018

linonetwo commented Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zionts Jul 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashtonsix commented Jul 10, 2018 • edited Loading

martijnwalraven commented Jul 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashtonsix commented Jul 11, 2018

Move `gui` option into constructor and use `playground` #1297

Move `gui` option into constructor and use `playground` #1297

zionts commented Jul 4, 2018 •

edited

Loading

evans commented Jul 5, 2018 •

edited

Loading

linonetwo commented Jul 7, 2018 •

edited

Loading

zionts Jul 9, 2018 •

edited

Loading

ashtonsix commented Jul 10, 2018 •

edited

Loading