Add server + route middleware functionality #5343
Conversation
🦋 Changeset detectedLatest commit: 3b12c41 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -14,7 +14,8 @@ | |||
| "main": "dist/index.js", | |||
| "typings": "dist/index.d.ts", | |||
| "dependencies": { | |||
| "@remix-run/node": "1.12.0" | |||
| "@remix-run/node": "1.12.0", | |||
| "@remix-run/router": "1.3.1" | |||
There was a problem hiding this comment.
Adapters will now include the router as a dependency so they can re-export MiddlewareContext/createMiddlewareContext alongside their own serverMiddleware concepts
packages/remix-express/server.ts
Outdated
| next: express.NextFunction | ||
| ) => { | ||
| let response: NodeResponse | undefined; | ||
| let context = createMiddlewareStore(); |
There was a problem hiding this comment.
It's worth noting that at the moment, since our context is keyed by object instances for strong-typing, users can run into cross-boundary CJS/ESM instance issues when using serverMiddleware. This is solveable by either (1) bundling your entire server via the remix.config.js server option, or (2) potentially using import maps in your application package.json (untested)
| } | ||
|
|
||
| export interface MiddlewareFunction { | ||
| (args: DataFunctionArgsWithMiddleware): DataFunctionReturnValue; |
There was a problem hiding this comment.
Let's bikeshed these names a bit - should they be LoaderFunction_V2, etc.?
| if ( | ||
| unstable_middleware !== true && | ||
| build.entry.module.handleDataRequest | ||
| ) { |
There was a problem hiding this comment.
Just like we will now ignore getLoadContext when you opt into middleware, we will also ignore handleDataRequest since you can operate on responses via the serverMiddleware. If folks need to decipher between document and data requests they should be able to do that via looking at response headers or the _data query param
| let request = createRemixRequest(req, res); | ||
| response = (await handleRequest(request)) as NodeResponse; | ||
| await sendRemixResponse(res, response); |
There was a problem hiding this comment.
If you turn on unstable_middleware and don't provide a serverMiddleware we just call through directly without any concept of getLoadContext
| if (!response) { | ||
| // User never called next(), so doesn't need to do any post-processing | ||
| response = await callRemix(); | ||
| } | ||
| if (!res.headersSent) { | ||
| await sendRemixResponse(res, response); | ||
| } |
There was a problem hiding this comment.
A few things going on here:
- If the user never calls
context.next()we automatically call the rest of the pipeline. You cannot short circuit by skipping the call. The only way to short circuit is tothrow - If the user actually responds via the express
Responsein the middleware (and thusres.headersSent === true) then we skipsendRemixResponse(). This allows them to take full control if they want to.
| "/parent-response", | ||
| "routes/parent-response" | ||
| ); | ||
| expect(response.headers.get("x-count")).toBe("2"); |
There was a problem hiding this comment.
These tests are OK, but I think you could be more explicit than a number. What about something like this?
expect(response.headers.get("x-processed-by")).toBe("parent")
…
expect(response.headers.get("x-processed-by")).toBe("parent,child")
…
expect(response.headers.get("x-processed-by")).toBe("parent,child,grandchild")
That makes it clearer which middleware are being invoked for each request.
|
Closing temporarily - we'll be revisiting shortly after v2 releases |
|
Now that V2 is out, is there a place to track this work? |
|
Yep! These are on the Roadmap as remix-run/react-router#12695 and #7645 |
Wire up Remix to new React Router middleware
Proposal: remix-run/react-router#9564
React Router PR: remix-run/react-router#9975
Note: CI will fail here until we can point to an experimental or prerelease version of the RR branch. They pass locally if you build the RR branch into the Remix
node_modulesdirectory.