Open jasonkuhrt opened 4 years ago
The order of middleware is important. For example authorization middleware should come after something like response-time middleware. If the auth fails, we don't want to lose response time checking!
Developers need to control manage this.
This is hard when plugins can add middleware.
We could add a z-index concept where plugins can state the priority of their middleware. An arbitrary int would be too unwieldy. But say it could be 1 or 2.
Nexus could guarantee that all level 1 gets run (so order does not matter). While level 2 would be potentially mutually exclusive to other middleware in its layer.
Layer 2 could be ordered by plugin name by default. Its deterministic.
If two plugins the user is using put middleware in an order that doesn't work for them the user could be given an api to customize it:
server.middlewareOrder.current // good for debugging
server.middlewareOrder.change(
'pluginA.middleware1',
'pluginB.middleware2',
'...rest',
)
If ordering needs to bring something to the top or bottom it could use the ...rest
arg.
varg function.
union of strings based on typegen (all middleware in use).
Each arg provided would remove itself from the union set those encoding set-like behaviour
Otherwise ordering changes would work by making relative changes
server.middlewareOrder.set(
'pluginA.middleware1',
'pluginB.middleware2',
)
Meaning here for example the location of pluginA.middleware1
does not change, and pluginB.middleware2
is put directly after it.
This would allow the user to organize the middleware in their app how they like, maybe distributed for some reason, while still being able to control the final order precisely. This aligns well with code-order-independence goal of Nexus.
Hi everybody,
I'm a newbie in software development. Recently I've been developing some very simple projects with feathersjs and I love the power of hooks to manage server-side logic.
Early this year @daffl launched repo to create an "Async middleware for JavaScript and TypeScript". Maybe this can be used in nexus middleware or this could also be a total non-sense.
Nevertheless it would be great to see a nexus-plugin-feathersjs.
Anyway, thanks for all your work.
Hey @osirvent, thanks for that very relevant share. Skimmed it but I'll need to take more time to understand why purity and functional composition with strong data primitives (like Either type) isn't good enough of a direction. fp-ts is probably leading the way in bringing these ideas to TS in a principaled way.
Happy to hear concrete examples of what we could leverage.
I do think getting paralyzed by analysis is a risk at this stage for us, and that starting to ship quickly and often is probably a good option right now for us to get traction.
This is a good feature. Support for accessing field and args as well would help. Nexus plugin as well can be used but that sounds complex for a regular simple use.
Any timeline for this as we are moving aggressively in adopting nexus-future. Is there any place to track the release notes for releases?
This is a good feature.
Which parts are most interesting to you? The more detail the better, its very helpful for us :)
Support for accessing field and args as well would help.
What does field
and args
mean here? Do you mean you want GraphQL info inside your server middleware?
Is there any place to track the release notes for releases?
GH releases soon. We just need https://github.com/prisma-labs/dripip/issues/21. CC @Weakky we can discuss prioritizing this again.
Thanks for your quick response. Here is more detailed description of the requirement.
In general here are the different middlewares I could see for a graphql api.
1 is more server level middleware. 2 and 3 are more graphql resolver level middlewares.
3 would be extremely helpful as it provides a layered architecture. It also helps with simplicity, configurability, reusability giving a good structure to code. After this middleware it would only be persistence using "t.crud...". If these middlewares can be enabled/disabled using some configuration it would be even more helpful. This helps to write a single codeline and use it for different applications, enable/disable middlewares depending on business need of the application.
Some of these are achievable in the old nexus using graphql-middleware. In nexus-future as we are using express, 2 and 3 have become a bottleneck. Nexus plugin could be a choice. This however seems to be requiring to be packaged separately as the codeline seems to be scanning package.json for something like nexus-plugin-... Loading from a local folder like "plugins" in similar to how we are loading from app.ts would be helpful. This however may need to be reviewed with your other larger goals in mind.
In case the above need can be achieved easily with other means, please suggest.
@paniavula I think 1 & 2 in your list can already be solved by Nexus Schema middleware:
But you seem to already be aware of this!
Nexus plugin could be a choice. This however seems to be requiring to be packaged separately as the codeline seems to be scanning package.json for something like nexus-plugin-... Loading from a local folder like "plugins" in similar to how we are loading from app.ts would be helpful. This however may need to be reviewed with your other larger goals in mind.
Yep we're very aware of this and want to address limitations around this soon!
What's not clear to me is, should the framework provide a higher level abstraction that makes writing middleware spanning server/schema easy, or, should users be left on their own to write server middleware + schema middleware to achieve their ends. I think this is tbd, and requires us to do the basics before we can get fancy, if at all.
Yep we're very aware of this and want to address limitations around this soon!
What's not clear to me is, should the framework provide a higher level abstraction that makes writing middleware spanning server/schema easy, or, should users be left on their own to write server middleware + schema middleware to achieve their ends. I think this is tbd, and requires us to do the basics before we can get fancy, if at all.
I think the current plugin architecture solves most of the immediate problems. The only critical need for now would be to write simple plugins locally and complex plugins through a separate dependency. The fancy stuff could also bloat the build size. My alpine based docker image for previous nexus (with prisma) was around 140MB when uncompressed. Now it is around 300+MB.
The fancy stuff could also bloat the build size
Totally, that's why we're working toward built in tree-shaking. We recently changed the plugin system to be tree-shakable. So those big worktime prisma deps will be droppable before shipping to prod.
The only critical need for now would be to write simple plugins locally
👌
Interesting discussion! Would this also account for the fact that you can currently only add Express middleware in your app.ts
before the setup by Nexus?
Specifically: I want a React app to be rendered on /*
, but obviously only when it doesn't concern a /graphql
request. When going through the source code it seems to me that the server.express
is a fresh server, and the ExpressGraphql
is only added later on. Would love to be able to add Express middleware after the Nexus routing is setup.
Would love to be able to add Express middleware after the Nexus routing is setup.
@ianwensink Interesting, I don't think this use-case is well documented/understood. Could you share more about why this is in a new issue?
Well, in Express I want to serve a static html file, but only if the request doesn't concern a Nexus request (/graphql
). This /graphql
route is added by Nexus, but after app.ts
is run. This means that I cannot add a wildcard route, since then all requests are caught by that wildcard.
To visualise, consider this:
/admin
/graphql
In Express, I would do this, semantically:
const express = new Server();
express.use('/admin', adminRouter);
express.use('/graphql
, expressGraphql);`express.get('/*', (res) => res.sendFile(file);
But due to the Nexus setup order, the order is actually: 1, 2, 4, 3.
A call to /graphql
will never reach number 3, because number 4 will catch everything. Now I have to add a "hack" to number 4 with req.path !== '/graphql'
, which works, but seems unnecessary.
Would love to hear your thoughts!
@ianwensink That is crystal clear thanks for the explanation!
The control flow proposed so far matches that of koa and feathers (among others surely).
But there is a major problem with the current design:
app level augmentation of req/res/ctx in middleware will not be visible in later middleware since the type of middleware can be altered based on syntactic order.
I will sketch some solutions to this.
First let's establish three middlwares to work with mentally:
function one(req){ req.fromOne = 1 /* somehow "fromOne" prop is added */ }
function two(req){ req.fromOne /* downstream, we want this to type check */ }
function three(req){ req.fromOne /* downstream, we want this to type check */ }
The problem is that in the chain, there is no encoded relationship between the links in the chain (middlewares).
// one two three are oblivious to one another
server.middleware(one)
server.middleware(two)
server.middleware(three)
We must encode the chain, there are two ways to do that:
server
.middleware(one)
.middleware(two)
.middleware(three)
// or
server.middlewares([one, two, three])
This allows us to type downstream functions based on upstream "contributions". An example of this chaining system can be seeing @Weakky's work on our internal test context system.
A problem with this solution though is that it does not support single-bodied before+after ware like so:
function one (req,res,next) {
// before
await next()
// after
}
AFAICT it is impossible to reconcile the static type transformation requirements with the above API. We would need new features in TypeScript. So, then, what could a new API look like?
server.on
.request(reqOne)
.request(reqTwo)
.request(reqThree)
.response(resOne)
.response(resTwo)
.response(resThree)
// or
server.middleware([reqOne,reqTwo,reqThree],[resOne,resTwo,resThree])
Expanding the API this way allows for before/after ware + type safety between steps.
Another problem is that by reserving return
for tracking req transformations we lose the previous semantic where return
was used for short-circuiting... to solve that we can again change the api, this time a middleware signature itself. The solution is to overload the next
param with an object containing next
and respond
. Now middleware can signal their intent.
function one(req, do) {
return do.next()
// or
return do.respond()
}
Aside: From a static point of view the do.res()
would result in a never
type being returned for downstream. In practice do.res()
would almost always surely be conditional, and never
is not something that can make much sense conditionally (nor is it possible anyways).
This problem looks irrevocable. Remember that middleware will be pluggable, and middleware order often matters, and since Nexus is a declarative system, it means we need a way for users both with respect to usable plugin system and their own apps to be able to declaratively swap middleware ordering.
How can this possibly be done in a system where we've laid out a tight static transformation???
// we've just invalidated what our static types tell us :( :( :(
server.middleware.ordering(
'two',
'one',
'three'
)
Even if one believes in-app middleware re-ordering is not an important feature, the plugin system use-case should be enough motivation.
This brings us to the end of the "Pure / Static Solution" idea, to another, typegen one...
This solution tries to deal with the original problems without the pitfalls of the static / pure solution, namely:
The first insight here is that all middleware needs names, to support middleware reordering. Let's build that into the API:
server.middleware('one', (req, res, ctx, next) => { req.a = 1; req.b = 2 })
server.middleware('two', (req, res, ctx, next) => { req.a /* should be type error! */ })
server.middleware('three', (req, res, ctx, next) => { req.b /* should be type safe! */ })
server.middleware.ordering(
'two',
'one',
'three'
)
With the minor change of names, now, we have the power to type the middleware according to typegen around the name (just like we do for Nexus schema and gql object type names). The full solution high level goes something like this:
req.headers
and context
data basically. Maybe could be expanded to general solution thoughWe could consider supporting an unnamed middleware:
server.middleware((req,res,ctx,next) => {...})
For some, maybe many, users, this would be enough. Middleware names are only important when an app has multiple of them.
With this solution, user might still need to reorder their one middleware with middlwares from plugins. So they still need a way to refer to their middleware. The default name could be app
.
If user wants to write their function in isolation we could make the typegen available for them so they can type their function:
const one: Nexus.Server.Middlewares.One = (req,res,ctx,next) => {...}
The one catch 22 here is that for this type to exist typegen must have run AND user must have done this:
server.middleware('one', ...)
Likely users won't internalize the order of operations here and just kind of write everything and wait to see it "working". And of course needing to do this, let alone understand the order of operations is absolutely a leak of internals. But it is minor, and it follows the gradual complexity goal ish since the simplest thing a user can do is write it inline. Extracting a func like this, and wanting it fully typed, could be seeing as taking on more complexity (again, ish).
Finally, we can create a middleground that provides value with just a bit less safety, a generic middleware type (notice, no s
):
const one: Nexus.Server.Middleware = (req,res,ctx,next) => {...}
This is slightly less safe but will always work.
AFAICT this solution looks close to ideal for a user. There are various ways it is made gradual, and it achieves the two goals we set out above:
Perceived Problem
server.custom
in every app to add express middlewareserver.custom
should be a rare escape hatchserver.custom
widespread use would make Nexus framework de-facto coupled to expressIdea
await next
to transition seemlessly from beforeware to afterwarereturn
to short-citcuitctx
, intentionally mimicks position in resolver sigschema.addToContext
app level augmentation of req/res/ctx in middleware will not be visible in later middleware since the type of middleware can be altered based on syntactic order.
[1] "for use at app level" because we don't want plugins to start depending on features from other plugins, or do we?
[2] We cannot use global interface merging b/c we don't know if the plugin will be enabled or disabled. We resort to typegen.
[3] They already can, but ctx is only for resolvers right now. This point is saying that plugins will be able to modify context for middleware too.
Ideas from other middleware systems we align with
koa:
fastify
Examples
Beforeware
Beforeware & Afterware
Beforeware short circuit
plugin augmnet req
Lifecycle
imagine three middlewares
normal
short-circuit ex 1
Notes
Alt: Dedicated before/afterware functions
Plugin-level type extraction
We will run type-extraction at the app level. Why not the plugin level too then?
Why not use an existing system
Links