feat(fastify): Integrate apollo-fastify plugin #626#1013
feat(fastify): Integrate apollo-fastify plugin #626#1013evans merged 35 commits intoapollographql:masterfrom
Conversation
|
@addityasingh: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
|
@martijnwalraven @abernix Can you please have a look at this PR? I don't know why the Node 4 and Node 6 builds are breaking as my changes shouldn't even affect those |
|
Strange, the error comes from Hapi.. 😕 |
|
@Extarys And I have been merging master to my branch because of periodically other changes being merged to master and the master still seems to be breaking for Node 4 and 6 |
|
:/ Huh... I installed fastify yesterday and I'm very excited about this merge - hope it'll eventually works. 😄 |
|
We got an issue here 😢 https://github.com/coopnd/fastify-apollo/issues/15 I converted my koa to fastify because it seemed a little faster but now I cannot continue de develop my app :( I was certain there was already a package for fastify 😨 |
abernix
left a comment
There was a problem hiding this comment.
Thanks for working on this. I think the problems you're having with Node.js 4 and Node.js 6 are likely due to the introduction of async / await which aren't natively supported on those versions of Node.js (8 was the first which natively supported them without having them transpiled to generators). Therefore, the tsconfig might be a place to start investigating.
| @@ -0,0 +1,18 @@ | |||
| { | |||
| "compilerOptions": { | |||
There was a problem hiding this comment.
This tsconfig.json should extends the root tsconfig.json, in a similar fashion as seen here.
There was a problem hiding this comment.
There is an existing problem at times with extending the tsconfig with fastify, wherein the version of @types/node starts to conflict. I opened an issue in fastify for this, but it was very flaky in behaviour on my machine. Will try to use the extend again and check
| @@ -0,0 +1,34 @@ | |||
| { | |||
There was a problem hiding this comment.
It doesn't seem necessary to introduce a graphql-server-fastify package. These are only kept around because they did exist at one point, but they are now deprecated. Let's just not include these and have apollo-server-fastify be the sole package.
There was a problem hiding this comment.
Sure. I will remove it
| @@ -0,0 +1,2915 @@ | |||
| # THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. | |||
There was a problem hiding this comment.
Please use npm's package-lock.json, rather than yarn.lock (for consistency with the rest of this repository).
| "postinstall": "lerna bootstrap", | ||
| "pretest": "npm run compile", | ||
| "test": "npm run testonly --", | ||
| "tdd": "npm run compile && mocha --full-trace --timeout 5000 ./test/tests.js", |
There was a problem hiding this comment.
Let's keep this as a separate concern outside of this PR since it's not directly related.
| description: Setting up Apollo Server with Fastify | ||
| --- | ||
|
|
||
| [](https://badge.fury.io/js/apollo-server-core) [](https://circleci.com/gh/apollographql/apollo-cache-control-js) [](https://coveralls.io/github/apollographql/apollo-server?branch=master) [](https://www.apollographql.com/#slack) |
There was a problem hiding this comment.
Aside from the fact that one of these badges is referring to apollo-cache-control-js (something which seems to be the case on the root README.md too and not something you've specifically introduced), let's not include these badges for now.
There was a problem hiding this comment.
Makes sense. I removed all the badges except the slack and apollo-server coverage. Once this gets published I can add the badges specific to fastify plugin
|
@abernix There was an issue with Hapi due to formatting which fixed the Node 6 issue. But the Node 4 issue seems to be totally unrelated to
I also tried committing the Logs from local machine
|
|
@evans After checking the issue #1088, I think it would be a major change in the features supported and looks great. But IMO, I think we should still be able to support |
|
@addityasingh We'll get this released in 1.4, thought it will most likely receive very little attention unless the package is included in apollo-server 2.x |
|
@evans Thanks for merging. I am working on the PR for v2.x . Probably will have something by the end of the weekend. Also a nice way to find this can be by updating repo label with |
* chore(deps): update dependency @adonisjs/bodyparser to v2.0.3 * docs: Fix typo in lambda package README (#999) * chore(deps): update dependency @types/aws-lambda to v8.10.3 * chore(deps): update dependency @types/node to v9.6.7 * chore(deps): update dependency koa to v2.5.1 * chore(deps): update dependency hapi to v17.4.0 (#1009) * chore(deps): update dependency sinon to v5 (#1010) * chore(deps): update dependency @types/node to v9.6.8 * chore(deps): update dependency sinon to v5.0.2 * Temporarily remove version 2. * Update meteor-theme-hexo to 1.0.9. * chore(deps): update dependency sinon to v5.0.3 * chore(deps): update dependency @types/node to v9.6.9 * Revert "Temporarily remove version 2." This reverts commit 6f50769. * Update package.json's hexo.version to 3.7.1. * chore(deps): update dependency @types/node to v9.6.11 * chore(deps): update dependency @types/node to v9.6.12 * chore(deps): update dependency hexo-server to v0.3.2 * chore(deps): update dependency sinon to v5.0.5 * chore(deps): update dependency sinon to v5.0.6 * chore(deps): update dependency sinon to v5.0.7 * chore(deps): update dependency @types/node to v9.6.14 * chore(deps): update dependency @types/sinon to v4.3.2 * bump version of subscriptions-transport-ws for graphiql The newer subscription client has a more forgiving default ka timeout * chore(deps): update dependency @types/node to v9.6.15 * chore(deps): update dependency @types/sinon to v4.3.3 * Fix typos in setup.md * fix graphiql docs for hapi * tests: Stop testing Node.js 4, which has reached the end of its LTS. (#1026) Node.js 4 will no longer be receiving any additional updates from the Node.js foundation, as per the expected LTS schedule[0] and their blog post: https://medium.com/the-node-js-collection/april-2018-release-updates-from-the-node-js-project-71687e1f7742 [0]: https://github.com/nodejs/LTS * chore(deps): update dependency @types/node to v9.6.18 * chore(deps): update dependency @types/aws-lambda to v8.10.4 * chore(deps): update dependency @types/restify to v5.0.8 * chore(deps): update dependency body-parser to v1.18.3 * chore(deps): update dependency koa-bodyparser to v4.2.1 * chore(deps): update dependency mocha to v5.2.0 * chore(deps): update dependency supertest to v3.1.0 * chore(deps): update dependency hapi to v17.5.0 * docs: add pointer to apollo server 2 documentation (#1083) * chore(deps): update dependency meteor-theme-hexo to v1.0.10 * chore(deps): update dependency @types/aws-lambda to v8.10.5 * chore(deps): update dependency sinon to v5.0.9 * chore(deps): update dependency sinon to v5.0.10 * chore(deps): update dependency @types/node to v9.6.19 * chore(deps): update dependency hapi to v17.5.1 * chore(deps): update dependency typescript to v2.8.4 * chore(deps): update dependency meteor-theme-hexo to v1.0.13 (#1118) * chore(deps): update dependency @types/aws-lambda to v8.10.6 * chore(deps): update dependency @types/node to v9.6.20 * koa: remove dist names from readme example (#1122) Since koa@2 has been published for a long time, and almost all the middlewares migrated to koa@2. It's no need to add extra dist names for koa and koa middlewares. * chore(deps): update dependency @types/koa to v2.0.46 * chore(deps): update dependency @types/mocha to v5.2.1 * chore(deps): update dependency @types/restify to v5.0.9 * chore(deps): update dependency @types/express to v4.16.0 * chore(deps): update dependency sinon to v5.1.0 * chore(deps): update dependency @types/node to v9.6.21 * chore(deps): update dependency sinon to v5.1.1 * docs: Add/update documentation README.md. In order to provide a more universal README.md across all documentation deployment repositories, and most importantly, to reference the so-called "documentation for the documentation". * chore(deps): update dependency meteor-theme-hexo to v1.0.14 (#1199) This Pull Request updates dependency [meteor-theme-hexo](https://github.com/meteor/meteor-theme-hexo) from `v1.0.13` to `v1.0.14` **Note**: This PR was created on a configured schedule ("after 10pm every weekday,before 5am every weekday" in timezone `America/Los_Angeles`) and will not receive updates outside those times. <details> <summary>Release Notes</summary> ### [`v1.0.14`](https://github.com/meteor/meteor-theme-hexo/blob/master/CHANGELOG.md#v1014) [Compare Source](meteor/meteor-theme-hexo@v1.0.13...v1.0.14) * Allow the "Edit on GitHub" button to work on "versioned" documentation. [PR #​80](`https://github.com/meteor/meteor-theme-hexo/pull/80`) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com). * Update renovate.json * Update renovate.json Hopefully these additional hours and the offset won't be problematic for anyone, but they should help me out some. We'll see! * Update renovate.json * chore(deps): pin dependency hexo-versioned-netlify-redirects to v1.0.7 (#1203) This Pull Request updates dependency `hexo-versioned-netlify-redirects` from `^1.0.7` to `v1.0.7` --- This PR has been generated by [Renovate Bot](https://renovatebot.com). * chore(deps): update dependency @types/chai to v4.1.4 * chore(deps): update dependency @types/koa-router to v7.0.29 * chore(deps): update dependency @types/mocha to v5.2.3 * chore(deps): update dependency @types/multer to v1.3.7 * chore(deps): update dependency @types/node to v9.6.22 * chore(deps): update dependency meteor-theme-hexo to v1.0.15 * chore(deps): update dependency @types/koa-router to v7.0.30 * chore(deps): update dependency @types/aws-lambda to v8.10.7 * chore(deps): update dependency hapi to v17.5.2 * Use apollo-server v1 in example relating to v1 * docs: Add sentence punctuation to 2.0 callout. Follows-up on #1083. * chore(deps): update dependency meteor-theme-hexo to v1.0.16 (#1262) This Pull Request updates dependency [meteor-theme-hexo](https://github.com/meteor/meteor-theme-hexo) from `v1.0.15` to `v1.0.16` <details> <summary>Release Notes</summary> ### [`v1.0.16`](https://github.com/meteor/meteor-theme-hexo/blob/master/CHANGELOG.md#v1016) [Compare Source](meteor/meteor-theme-hexo@v1.0.15...v1.0.16) * Re-introduce the scrolling ability within search results. [PR #​83](`https://github.com/meteor/meteor-theme-hexo/pull/83`) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com). * chore(deps): update dependency @types/mocha to v5.2.4 * chore(deps): update dependency multer to v1.3.1 * chore(deps): update dependency @types/node to v9.6.23 * chore(deps): update dependency apollo-hexo-config to v1.0.8 (#1329) This Pull Request updates dependency [apollo-hexo-config](https://github.com/apollographql/apollo-hexo-config) from `v1.0.7` to `v1.0.8` <details> <summary>Release Notes</summary> ### [`v1.0.8`](https://github.com/apollographql/apollo-hexo-config/compare/v1.0.7...v1.0.8) [Compare Source](https://github.com/apollographql/apollo-hexo-config/compare/v1.0.7...v1.0.8) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com). * docs: Add browser auto-reloading on source content changes. (#1336) By virtue of a relatively simple `hexo-browsersync` package[0], which implements BrowserSync[1] in Hexo, this change brings support for automatically reloading the browser when the source content has changed. No more pressing "Reload" in order to see the changes to the Markdown source when working on documentation! 🎉 [0]: https://npm.im/hexo-browsersync [1]: https://www.browsersync.io * chore(deps): update dependency koa to v2.5.2 * docs: add note about passing context as a function (#757) * docs: add note about passing context as a function We realized today (by mistake) that the value of `context` in `GraphQLOptions` can be a function. Adding a note to the docs so it doesn't surprise anyone else. * docs: update context as a function docs - fix description per @n1ru4l's feedback - add a code example of instantiating a new class in the context for each request * Added option to disable rewriting URL for GraphiQL (#1047) * added option to disable rewriting url for graphiql * updated docs * added link to PR in changelog * Add skipValidation option. (#839) * Don't validate if query is already an AST. * Skip validation option. * fix(hapi16_support): add hapi 16 next() invocation [closes #744] (#743) * chore(hapi16_support): add hapi 16 next() invocation * run lint fix * update changelog * feat(fastify): Integrate apollo-fastify plugin (#1013) * Integrate apollo-fastify plugin #626 * #626 Fix typescript issues * #626 Update changelog * #626 Update README * #626 Fix the breaking tests * #626 Fix code review comments * #626 Run Hapi tests only for node 8 and 9 * #626 Run Hapi tests only for node 8 and 9 * #626 Commit package.lock in working state * #626 Use npm instead of yarn for node 4 * Revert package-lock and circle ci test job steps * #626 Bump the version * v1.4.0 * bring version-2 up to date * remove apollo-server-fastify * fix export of hapi middleware * remove setup * fix hapi readme * remove unused tests
|
Looks like this PR was removed for releasing purposes? c00918b |
|
@evans I found out that this package name was used by someone else already and requested npmjs support to transfer it to me. I just got a reply from them about the transfer confirmation. Can we re-merge this now, if this was the reason to remove the package in the first place? |
|
I have started now also on v2.x so that we can publish that one asap as well |
|
@addityasingh 🎊 That's excellent! Thanks for working out the package ownership problem with npm — much appreciated. I suspect publishing problems due to ownership were the cause of the reversal in c00918b (which was reverted in a pinch). That said, to have this package be a part of this Just to be clear, this would require that you transfer the package to If that sounds okay, please go ahead and open a PR for the 2.0-enabled version and add the |
| const app = fastify(); | ||
|
|
||
| // jsonParser is needed for POST. | ||
| app.addContentTypeParser('application/json', function(req, done) { |
There was a problem hiding this comment.
Hi! I'm one of the Fastify maintainers :)
Thank you for implementing this!
Just a nit, you don't need to add a content type parse for application/json, since it's included directly in Fastify!
Besides Fastify performs many security checks that fast-json-body is not doing.
There was a problem hiding this comment.
@delvedor Thanks a lot for the pointer :) But I implemented this 4 months back and for some reason I had to explicitly add the content parser. But I am working on V2 of this plugin and will take your point into consideration while doing that
|
Can we implement |
|
try fastify-apollo-step ..
maybe try fastify-apollo-step.. |
|
@here Adding here for people who come across this thread: This is the link to the new PR to integrate with V2 WIP PR for apollo-fastify-plugin v2. Currently some tests are failing and I am trying to figure them out. Feel free to add your comments/suggestions. |

Refer #626
TODO:
closes #626