docs: add tutorial to expose GraphQL APIs #1899

dhmlau · 2018-10-22T20:18:42Z

Based on the QuickLab we've created for Node+JS Interactive, this PR added the GraphQL APIs part as one of the tutorials.

jannyHou · 2018-10-22T20:26:57Z

docs/site/exposing-graphql-api.md

+lb4 example todo
+```
+
+### Install OASGraph and required dependencies


nitpick: all upper case?

OK. Will change it.
Actually our docs are not quite inconsistent on the capitalization of the titles. :(

jannyHou

LGTM

marioestradarosa · 2018-10-22T21:40:19Z

@dhmlau,

I think this line is missing from inside the src/application.ts file, or can it be resolved differently?

options = Object.assign({}, {
       rest: {
         openApiSpec: {
           servers: [{url:'http://localhost:3000'}],
         }
       }
    });

otherwise, the query in the doc will return

{
  "errors": [
    {
      "message": "Invalid URI \"/todos\"",
      "locations": [

dhmlau · 2018-10-23T00:23:52Z

Thanks @marioestradarosa for catching that. Somehow I thought it's no longer needed! PTAL again.

marioestradarosa · 2018-10-23T01:32:25Z

Thanks @marioestradarosa for catching that. Somehow I thought it's no longer needed! PTAL again.

@dhmlau , I have just open a PR #1900 that I think can make things easier for the application in this matter. If it lands, then I believe we should then modify the examples accordingly.

thoughts?

bajtos

Looks mostly good.

bajtos · 2018-10-23T12:00:05Z

docs/site/exposing-graphql-apis.md

+---
+lang: en
+title: 'Exposing GraphQL APIs'
+keywords: LoopBack 4.0, LoopBack 4


Should we add "GraphQL" keyword too?

bajtos · 2018-10-23T12:03:43Z

docs/site/exposing-graphql-apis.md

+and the required dependencies:
+
+```sh
+npm i --save oasgraph express-graphql


I am curious, why do we have to install express-graphql ourselves? I would expect the CLI command oasgraph to be self-contained and come with all dependencies.

The OASGraph is meant to be used as a library to transform from OpenAPI to GraphQL schema, It doesn't need express-graphql, since you can use any other package once you have your schema converted. The main idea of the library is to be used inside the application.

However the CLI is used to test the implementation and it requires express-graphql. It is not a separate package (like monorepo), it is just a single file that implements the GraphQL server with the generated schema. IMO, it is not meant for production.

Thanks @marioestradarosa. @dhmlau has pointed me to IBM/openapi-to-graphql#30, I have a better understanding now.

So, assuming oasgraph is not going to change, I am proposing the following changes:

Include express in the modules to install, we must not rely on express being installed in top-level node_modules by being a dependency of @loopback/rest.

$ npm i --save oasgraph express express-graphql

Explain the users why we are asking them to install those two extra express modules and point them to the discussion in IBM/openapi-to-graphql#30 to learn more and/or give us feedback.

docs/site/exposing-graphql-apis.md

bajtos · 2018-10-23T12:08:27Z

docs/site/exposing-graphql-apis.md

+The expected output looks like this:
+
+```json
+{ "data": { "todos": [ { "id": 1, "title":


Can you reformat the data to make it easier to read please? E.g. use JSON.stringify(data, null, 2), or perhaps put each todos array item on its own line.

{ "data": { "todos": [ {"id": 1, /*...*/}, {"id": 2, /*...*/}, // etc. ]}}

bajtos · 2018-10-23T12:09:54Z

docs/site/sidebars/lb4_sidebar.yml

@@ -98,6 +98,11 @@ children:
    - title: 'Run and Test it'
      url: soap-calculator-tutorial-run-and-test.html
      output: 'web, pdf'
+
+  - title: 'Exposing GraphQL APIs'


Is this still a Tutorial? I feel that "Exposing GraphQL APIs" and "Deploying to IBM Cloud" don't belong to Tutorials section, I would certainly not look for these pages in Tutorials myself!

Can we introduce a new section "Guides" please and move both pages there?

How about turning them into a How to sections ?

How to sounds good to me.

marioestradarosa · 2018-10-23T15:00:56Z

@dhmlau, according to PR #1900 and @raymondfeng suggestions, here is the final code for the application.ts, this will populate the server property automatically.

options = Object.assign(
      {
        rest: {
          openApiSpec: {
            setServersFromRequest: true,
          },
        },
      },
      options,
    );

bajtos · 2018-10-23T15:41:03Z

Hmm, we may want to use a deep-assign implementation instead of Object.assign- Object assign will overwrite the entire rest configuration with the object provided by the caller.

raymondfeng · 2018-10-23T15:51:42Z

docs/site/exposing-graphql-apis.md

+command:
+
+```sh
+node_modules/.bin/oasgraph http://localhost:3000/openapi.json


This can be simplified as npx oasgraph http://localhost:3000/openapi.json

Just need to mention the user that npx comes bundled with npm version >5.2 . it works fine, I have just tested, didn't know about npx 😄

Node.js 8.9.0, the minimal Node.js version required by LoopBack 4, comes with npm version 5.5.1, which includes npx. IMO, there is no need to explain how to run oasgraph on npm version older than 5.2.

Having said that, I think most LoopBack users are probably not familiar with npx yet, so we should give them a pointer on where to learn more.

For example, add the following paragraph after the code snippet showing how to invoke npx:

_Haven't heard about `npx` yet? It's a cool helper provided by `npm` and available out of the box since Node.js 8.x. Learn more in their announcement blog post: [Introducing npx: an npm package runner](https://medium.com/@maybekatz/introducing-npx-an-npm-package-runner-55f7d4bd282b))_

marioestradarosa · 2018-10-23T16:01:59Z

Object assign will overwrite the entire rest configuration with the object provided by the caller.

I thought that adding options as the end parameter prevents this overwrite. I did some tests.

If we place the options as the second parameter as it is here in this comment, then the config that comes in by the caller takes precedence.

On the other hand, if we place it as the first parameter, the rest property defined here in applications.ts takes precedence over the the one that is coming from the caller as follows:

options = Object.assign(
    options,
      {
        rest: {
          openApiSpec: {
            setServersFromRequest: true,
          },
        },
      },
    );

Which one is better for this case?.

dhmlau · 2018-10-24T15:02:57Z

Thanks for your feedback! To move this PR forward, I'm going to incorporate the feedback with the major changes:

Remove Customize the Server URL of the REST Server section to modify the application.ts after @marioestradarosa's PR (refactor(cli): use options to specify port and OpenAPI server url #1900 )has landed
- I think the PR includes the template changes in the application.ts, and supposedly users don't need to do anything else.
Move the instructions for GraphQL (this PR) and "Deploying to Cloud" to a new section How Tos.

~~Will create a new task for having the oasgraph and LB rest server running on the same port.~~
EDITED: Created #1905 for having the oasgraph and LB rest server running on the same port.

bajtos · 2018-10-25T10:28:22Z

docs/site/exposing-graphql-apis.md

+command:
+
+```sh
+node_modules/.bin/oasgraph http://localhost:3000/openapi.json


Node.js 8.9.0, the minimal Node.js version required by LoopBack 4, comes with npm version 5.5.1, which includes npx. IMO, there is no need to explain how to run oasgraph on npm version older than 5.2.

Having said that, I think most LoopBack users are probably not familiar with npx yet, so we should give them a pointer on where to learn more.

For example, add the following paragraph after the code snippet showing how to invoke npx:

_Haven't heard about `npx` yet? It's a cool helper provided by `npm` and available out of the box since Node.js 8.x. Learn more in their announcement blog post: [Introducing npx: an npm package runner](https://medium.com/@maybekatz/introducing-npx-an-npm-package-runner-55f7d4bd282b))_

docs/site/exposing-graphql-apis.md

bajtos

👍

marioestradarosa · 2018-10-26T13:29:04Z

@dhmlau , I think your PR is now ready to land 👍

dhmlau requested review from bajtos and raymondfeng as code owners October 22, 2018 20:18

jannyHou reviewed Oct 22, 2018

View reviewed changes

jannyHou approved these changes Oct 22, 2018

View reviewed changes

dhmlau force-pushed the graphql branch 2 times, most recently from be7dcc2 to cb77b58 Compare October 23, 2018 00:22

bajtos reviewed Oct 23, 2018

View reviewed changes

raymondfeng reviewed Oct 23, 2018

View reviewed changes

dhmlau mentioned this pull request Oct 24, 2018

Make REST and GraphQL server running on the same port #1905

Closed

3 tasks

bajtos reviewed Oct 25, 2018

View reviewed changes

dhmlau force-pushed the graphql branch from 97a7695 to 2b4451d Compare October 25, 2018 13:55

bajtos approved these changes Oct 25, 2018

View reviewed changes

dhmlau force-pushed the graphql branch from 2b4451d to 2679ba0 Compare October 26, 2018 18:15

docs: add tutorial to expose GraphQL APIs

2679ba0

raymondfeng approved these changes Oct 26, 2018

View reviewed changes

dhmlau merged commit 9fd37c9 into master Oct 26, 2018

dhmlau deleted the graphql branch October 26, 2018 18:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add tutorial to expose GraphQL APIs #1899

docs: add tutorial to expose GraphQL APIs #1899

dhmlau commented Oct 22, 2018

jannyHou Oct 22, 2018

dhmlau Oct 22, 2018

jannyHou left a comment

marioestradarosa commented Oct 22, 2018

dhmlau commented Oct 23, 2018

marioestradarosa commented Oct 23, 2018

bajtos left a comment

bajtos Oct 23, 2018

bajtos Oct 23, 2018

marioestradarosa Oct 23, 2018

bajtos Oct 23, 2018

bajtos Oct 23, 2018

bajtos Oct 23, 2018

marioestradarosa Oct 23, 2018

dhmlau Oct 23, 2018

marioestradarosa commented Oct 23, 2018

bajtos commented Oct 23, 2018

raymondfeng Oct 23, 2018

marioestradarosa Oct 23, 2018

bajtos Oct 25, 2018

marioestradarosa commented Oct 23, 2018 •

edited

Loading

dhmlau commented Oct 24, 2018 •

edited

Loading

bajtos Oct 25, 2018

bajtos left a comment

marioestradarosa commented Oct 26, 2018

docs: add tutorial to expose GraphQL APIs #1899

docs: add tutorial to expose GraphQL APIs #1899

Conversation

dhmlau commented Oct 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou left a comment

Choose a reason for hiding this comment

marioestradarosa commented Oct 22, 2018

dhmlau commented Oct 23, 2018

marioestradarosa commented Oct 23, 2018

bajtos 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioestradarosa commented Oct 23, 2018

bajtos commented Oct 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marioestradarosa commented Oct 23, 2018 • edited Loading

dhmlau commented Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

marioestradarosa commented Oct 26, 2018

marioestradarosa commented Oct 23, 2018 •

edited

Loading

dhmlau commented Oct 24, 2018 •

edited

Loading