Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSR node middleware not working on Express #7870

Closed
1 task
marsidev opened this issue Jul 30, 2023 · 5 comments · Fixed by #8176
Closed
1 task

SSR node middleware not working on Express #7870

marsidev opened this issue Jul 30, 2023 · 5 comments · Fixed by #8176
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: ssr Related to SSR (scope) pkg: node Related to Node adapter (scope)

Comments

@marsidev
Copy link

What version of astro are you using?

2.9.6

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

pnpm

What operating system are you using?

Windows

What browser are you using?

Chrome

Describe the Bug

When enabling SSR (output: "server" or output: "hybrid"), with a middleware node adapter, the middleware is not working on Express (v4.18.2) when using app.use(ssrHandler), as shown in the docs:

image

Instead, it works when using the middleware as follows:

app.use((req, res, next) => {
  ssrHandler(req, res, next);
});

I noticed it works as expected on Fastify (v4.21.0) with just app.use(ssrHandler).

This is my config:

export default defineConfig({
  output: "server",
  adapter: node({
    mode: "middleware",
  }),
  server: {
    port: 8000,
  },
});

This is my express server:

import express from "express";
import { handler as ssrHandler } from "./dist/server/entry.mjs";

const PORT = process.env.PORT || 8000;
const app = express();

app.use(express.static("dist/client/"));
app.use(ssrHandler); // <-- This is not working

// The following works:
// app.use((req, res, next) => {
//   ssrHandler(req, res, next);
// });

app.listen(PORT, () => {
  console.log(`Server running on http://localhost:${PORT}`);
});

I created a repo to reproduce the error:

  1. Fork/clone the repo
  2. Install deps (pnpm i)
  3. Build the project (pnpm build)
  4. Run express server (pnpm preview)
  5. Go to http://localhost:8000/ssr or http://localhost:8000/api/ssr to check if works

What's the expected result?

It should work with app.use(ssrHandler).

Please fix it or at least update the docs.

Link to Minimal Reproducible Example

https://github.com/marsidev/astro-ssr-example

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jul 30, 2023
@andremralves
Copy link
Contributor

It stopped working in version v5.3.0 when the locals parameter was added to the middleware function. #7385

Apparently app.use() can't handle functions with an extra parameter directly.

@ematipico ematipico added - P4: important Violate documented behavior or significantly impacts performance (priority) pkg: node Related to Node adapter (scope) and removed needs triage Issue needs to be triaged labels Jul 31, 2023
@Princesseuh Princesseuh added the feat: ssr Related to SSR (scope) label Aug 1, 2023
@yaronovsky
Copy link

When will this be addressed? It prevents me from updating node adapter

@yaronovsky
Copy link

Any news here? Not getting addresses I see

@matthewp
Copy link
Contributor

Can this be fixed or should we update the docs?

@yaronovsky
Copy link

Thank you so much, it really matters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: ssr Related to SSR (scope) pkg: node Related to Node adapter (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants