fix: update to support hapi 17 by Saeris · Pull Request #396 · graphql/graphql-playground

Saeris · 2018-01-04T13:20:28Z

Hapi 17 introduces breaking changes to it's API which affects all existing plugins. This fix is intended to migrate from hapi 16 to 17. Currently graphql-playground-middleware-hapi will throw an error such as the following:

error: AssertionError [ERR_ASSERTION]: Invalid plugin options {
  "plugin": {
    "register": function (server, options, next) {\n    if (arguments.length !== 3) {\n        throw new Error(\"Playground middleware expects exactly 3 arguments, got \" + arguments.length);\n    }\n    var path = options.path, _a = options.route, config = _a === void 0
 ? {} : _a, rest = __rest(options, [\"path\", \"route\"]);\n    var middlewareOptions = __assign({}, rest, { version: pkg.version });\n    server.route({\n        method: 'GET',\n        path: path,\n        config: config,\n        handler: function (request, reply) {\n
            reply(graphql_playground_html_1.renderPlaygroundPage(middlewareOptions)).header('Content-Type', 'text/html');\n        },\n    });\n    return next();\n},
    "options": {
      "path": "/playground",
      "endpoint": "/graphql"
    },
    "name" [1]: -- missing --
  }
}

[1] "name" is required

Changes:

Updated graphql-playground-middleware-hapi plugin definition to support hapi 17.

Removed attributes?: any and replaced it with pkg?: any. multiple is false by default in hapi 17.

Please see the following for more information on the changes between hapi 16 and 17:

https://futurestud.io/tutorials/hapi-v17-upgrade-guide-your-move-to-async-await
https://hapijs.com/api#plugins

Updated the example project to use hapi 17, including dependencies.

Please change package version numbers as required.

Updated plugin definition to support hapi 17. Please see the following for more information on the changes between hap 16 and 17: https://futurestud.io/tutorials/hapi-v17-upgrade-guide-your-move-to-async-await https://hapijs.com/api#plugins Updated the example project to use hapi 17, including dependencies.

CLAassistant · 2018-01-04T13:20:35Z

All committers have signed the CLA.

Saeris · 2018-01-04T13:22:02Z

+  } catch (err) {
+    console.log(`Failed to start server!`, err)
  }
-  console.log(`Server running at: ${server.info.uri}`)


Whoops! This line can be deleted!

schickling

Thanks a lot for updating this!

schickling · 2018-01-04T13:29:39Z

@timsuchanek can you merge & publish this?

Saeris · 2018-01-04T13:37:30Z

I'll be heading out of town later today and won't be available again until about the middle of next week. As suggested I'll check out npm publish when I get back to test this and update you then.

Saeris · 2018-01-04T23:56:28Z

quick update:

I found a bit of time to try publishing it to my scope to test as you suggested, found I needed to make some more changes.

Running into an issue with the middleware now where it can't find the css/middleware.js on the CDN and I think it's because of a version number mismatch. I'll look into a fix later on.

Seems likely that because the mono-repo version numbers need to be in-sync to dynamically grab the version number from the package.json file, that's one part of the problem since my version of the package will always been out of sync with the mono-repo.

I can test to make sure the middleware works I think by using a specific version number that's on the CDN. For now I can at least get the page to show a loading indicator. Here's the console errors from Chrome for reference:

GET http://cdn.jsdelivr.net/npm/graphql-playground-react@1.3.6/build/static/css/index.css net::ERR_ABORTED
GET http://cdn.jsdelivr.net/npm/graphql-playground-react@1.3.6/build/static/js/middleware.js net::ERR_ABORTED
Uncaught ReferenceError: GraphQLPlayground is not defined
    at playground:531

I'll push my WIP in case anyone has the time to take a look.

Work in progress commit for review + feedback.

Saeris · 2018-01-05T00:00:50Z

 export interface Register {
-  (server: Server, options: MiddlewareOptions, next: any): void
-  attributes?: any
+  (server: Server, options: MiddlewareOptions): void


Not too familiar with Typescript, so if I'm doing this wrong please let me know!

timsuchanek

Thanks for the PR @Saeris and sorry for getting back so late!
There are 2 small changes that I would like to see before I can merge the it.

timsuchanek · 2018-01-18T10:40:02Z

 const hapi = require('hapi')
 const { graphqlHapi } = require('apollo-server-hapi')
-const hapiPlayground = require('graphql-playground-middleware-hapi').default
+const hapiPlayground = require('@saeris/graphql-playground-middleware-hapi').default


Please change this to ../../dist/index

timsuchanek · 2018-01-18T10:41:19Z

+      method: 'GET',
+      path,
+      config,
+      handler: async (request, h) => h.response(await renderPlaygroundPage(middlewareOptions)).type('text/html')


With the latest graphql-playground-html there is no need for async await anymore

@timsuchanek

Made changes requested by @timsuchanek Plugin now uses key `playgroundVersion` in `package.json` to get middleware version from jsdelivr. Examples/basic now uses the local version of the middleware as built with `yarn build` located in `dist`. Removed dependency from example `package.json`, as it is no longer used. Set example version to `2.0.0` as it now uses Hapi 17. Updated middleware dependencies to reflect those of the current version. Middleware version number will need to be bumped to a new major version for the next release, as it will not be backwards compatible with Hapi 16. TODO: Update readme to direct users to use the last stable version supporting Hapi 16 if they don't wish to upgrade.

timsuchanek

Thanks for putting the work into this!

@timsuchanek

* fix: update to support hapi 17 Updated plugin definition to support hapi 17. Please see the following for more information on the changes between hap 16 and 17: https://futurestud.io/tutorials/hapi-v17-upgrade-guide-your-move-to-async-await https://hapijs.com/api#plugins Updated the example project to use hapi 17, including dependencies. * v2.0.1 * v2.0.2 * v2.0.3 * v2.0.4 * v2.0.5 * wip: hapi-17-update fixes Work in progress commit for review + feedback. * refactor: get middleware version from package.json, example use local Made changes requested by @timsuchanek Plugin now uses key `playgroundVersion` in `package.json` to get middleware version from jsdelivr. Examples/basic now uses the local version of the middleware as built with `yarn build` located in `dist`. Removed dependency from example `package.json`, as it is no longer used. Set example version to `2.0.0` as it now uses Hapi 17. Updated middleware dependencies to reflect those of the current version. Middleware version number will need to be bumped to a new major version for the next release, as it will not be backwards compatible with Hapi 16. TODO: Update readme to direct users to use the last stable version supporting Hapi 16 if they don't wish to upgrade.

Saeris commented Jan 4, 2018

View reviewed changes

schickling approved these changes Jan 4, 2018

View reviewed changes

Drake Costa added 5 commits January 4, 2018 15:25

v2.0.1

e30d3a7

v2.0.2

7db3637

v2.0.3

80d7690

v2.0.4

ea97c74

v2.0.5

b847d3f

wip: hapi-17-update fixes

a71168d

Work in progress commit for review + feedback.

Saeris commented Jan 5, 2018

View reviewed changes

timsuchanek suggested changes Jan 18, 2018

View reviewed changes

timsuchanek mentioned this pull request Jan 18, 2018

Request for hapi 17 support #233

Closed

2 tasks

Drake Costa added 2 commits January 19, 2018 00:03

Merge branch 'master' into hapi-17-update

6bfa6eb

timsuchanek approved these changes Jan 19, 2018

View reviewed changes

timsuchanek merged commit a4ddbd6 into graphql:master Jan 19, 2018

Saeris deleted the hapi-17-update branch January 19, 2018 10:58

Navigation Menu

Uh oh!

fix: update to support hapi 17 - #396

fix: update to support hapi 17#396
timsuchanek merged 9 commits into
graphql:masterfrom
Saeris:hapi-17-update

Saeris commented Jan 4, 2018

Uh oh!

CLAassistant commented Jan 4, 2018 •

edited

Loading

Uh oh!

Saeris Jan 4, 2018

Uh oh!

schickling left a comment

Uh oh!

schickling commented Jan 4, 2018

Uh oh!

Saeris commented Jan 4, 2018

Uh oh!

Saeris commented Jan 4, 2018

Uh oh!

Saeris Jan 5, 2018

Uh oh!

timsuchanek left a comment

Uh oh!

timsuchanek Jan 18, 2018

Uh oh!

timsuchanek Jan 18, 2018

Uh oh!

timsuchanek left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Saeris commented Jan 4, 2018

Uh oh!

CLAassistant commented Jan 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Saeris Jan 4, 2018

Choose a reason for hiding this comment

Uh oh!

schickling left a comment

Choose a reason for hiding this comment

Uh oh!

schickling commented Jan 4, 2018

Uh oh!

Saeris commented Jan 4, 2018

Uh oh!

Saeris commented Jan 4, 2018

Uh oh!

Saeris Jan 5, 2018

Choose a reason for hiding this comment

Uh oh!

timsuchanek left a comment

Choose a reason for hiding this comment

Uh oh!

timsuchanek Jan 18, 2018

Choose a reason for hiding this comment

Uh oh!

timsuchanek Jan 18, 2018

Choose a reason for hiding this comment

Uh oh!

timsuchanek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Jan 4, 2018 •

edited

Loading