graphile / crystal

🔮 Graphile's Crystal Monorepo; home to Grafast, PostGraphile, pg-introspection, pg-sql2 and much more!
https://graphile.org/
Other
12.63k stars 572 forks source link

Documentation / Example for "Wrapping an existing resolver" #587

Closed edqallen closed 7 years ago

edqallen commented 7 years ago

I have a system using postgraphile as a library in an express application and so far things are working pretty well, but I'm hitting a snag. I want to do some input validation for mutations within express -- not via triggers/constraints/etc within pg. My thought was to do this via a plugin as documented here: https://www.graphile.org/postgraphile/extending/#wrapping-an-existing-resolver

However, the example is incomplete. In particular there are a few points where the example code says ...rest indicating (to me, I may be mistaken) that I am supposed to, for example, "Copy over everything except 'resolve'". Since postgraphile is itself generating all the mutations, I'm at a loss as to where to find just what I need to copy over here.

I'm a fairly experienced developer, but it should be made obvious that I'm extremely new to node, express, and graphQL, so excuse me if I'm missing something obvious. As is, the existing example simply doesn't work, though I don't expect it to. Unexpected token ... makes complete sense, though at first I thought it might be some fancy new expression from typescript.

Edit: Turns out that ... is in fact some sort of language construct, so I'm not sure why the example code is complaining about it.

benjie commented 7 years ago

My apologies for being lazy in the docs and using the stage three object spread syntax - it's not officially in the language yet (hopefully it'll be in ES2018) so I should have done that with Object.assign instead.

https://github.com/tc39/proposal-object-rest-spread/blob/master/README.md

Nonetheless, I be

benjie commented 7 years ago

Ugh, mobile...

Nonetheless I believe you can use the syntax on Node v8.5+

Failing that you could transpile your code with Babel or switch to using Object.assign for spreading and maybe something like lodash's "omit" to get an object with some keys omitted.

chadfurman commented 7 years ago

@edqallen I'm working on an example boilerplate here: https://github.com/chadfurman/rpg-boilerplate

I'm using some other things which might be confusing (i.e. react, relay, etc), however the webpack config in the frontend/config folder might be useful to you.

I'll be adding in documentation and testing/debugging it today.

chadfurman commented 7 years ago

Update: Example is in place. I've included a plugin here: https://github.com/chadfurman/rpg-boilerplate/blob/master/api/src/plugins/PluginName.js

You can use this boilerplate to test out additional plugins, as well!

@benjie I suggest we close this ticket, and we can re-open it if he needs us?

benjie commented 7 years ago

I think the issue relates more to the use of ES2018 syntax in the docs, which still requires solving.

edqallen commented 7 years ago

I haven't forgotten or abandoned this issue, I just haven't had time until now to look at your responses in detail. We are currently using node 6 for our server, since that is the current LTS version. It sounds like that is what's preventing the existing examples from working, and may prevent @chadfurman 's from working as well?

My goal here is simply to provide some data validation before the mutations hit pg, as we aren't really keen on doing it in the db server. I assume there isn't some other way to accomplish this with postgraphql at present.

Edit: Upping to node 8.6 did solve the issue with the examples not working. I'm still wondering if this is the "right" way to do input validation outside of PG, but that should probably be a separate issue.

chadfurman commented 7 years ago

Node 6 will require a transpiler for ES6 features.

Client-side data validation is outside the scope of PostgraphQL -- however, if you're using the RPG Boilerplate (which uses docker containers and bundles node 8.5), then please open a ticket in that repo and we can discuss.

In general, if you don't want to use constraints and checks on fields in your DB (you probably should -- database-level data integrity verification is sexy), and if you don't want to extend the GraphQL schema by writing a plugin in node that does your filtering for you, then yes you have to do the filtering on the front-end in whatever client-side JS framework you're using.

chadfurman commented 7 years ago

I see you've started a discussion over in #590 about input validation.

@benjie https://github.com/graphile/graphile.github.io/issues/22 ticket created for docs w.r.t. ES2018

benjie commented 7 years ago

[semi-automated message] To keep things manageable I'm going to close this issue as I think it's solved; but if not or you require further help please re-open it.

edqallen commented 7 years ago

Closing is fine by me. @chadfurman I do want to do the validation server side, just not in the PG server. We don't really want to use anything more complex than simple constraint checks on the PG side since maintaining and testing them is somewhat difficult, adding or modifying them requires a db migration, and the node server scales easier than the pg server. Some of our use cases involve calculating a field on insert/update using a potentially long lookup from a remote service, which isn't something I'd want to burden a pg backend process with.

What I'm looking for in #590 is then a way to get it done in the express server that is hosting postgraphql, and I believe the only practical way to do this is via the postgraphql plugin system.

chadfurman commented 7 years ago

yes, server-side validation with the v4 plugin system sounds like what you need :)

korenzerah commented 5 years ago

@edqallen It's been a while, but if you could share what you've done in order to implement input validation on server side, it'll be nice :) Thanks ahead.

benjie commented 5 years ago

(Check out https://github.com/graphile/operation-hooks )