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

add ability to pass middleware to @astrojs/node when using standalone… #11516

Closed

Conversation

nm123github
Copy link

@nm123github nm123github commented Jul 20, 2024

Add option to pass middleware to @astrojs/node when using standalone mode.

Changes

  • Make changes in the @astrojs/node to accept standaloneMiddleware file as a parameter
  • If the standaloneMiddleware param is passed, @astrojs/node (in standalone mode) will attempt to load the middleware's and run them.
  • The middleware file is expected to return the combined list of middleware's that need to run.
  • The standalone server will work as it works today, if no standaloneMiddleware option is passed.

Testing

I've tested this using a standalone app. Also added tests. Below are the sample changes.

astro.config.mjs

import { defineConfig } from "astro/config";
import node from "@astrojs/node";

// https://astro.build/config
export default defineConfig({
  output: "server",
  adapter: node({
    mode: "standalone",
    standaloneMiddleware: `${import.meta.dirname}/src/node_middleware.js`,
  }),
  integrations: [{
    name: "astro-lifecycle-logs",
    hooks: {
      'astro:server:setup': async ({ server }) => {
        // used during `pnpm run dev`
        const middleware = await import(`${import.meta.dirname}/src/node_middleware.js`);
        server.middlewares.use(middleware.default);
      }
    }
  }],
});

node_middleware.js

const combineMiddleware = (...middlewares) => {
    return (req, res, next) => {
        let index = 0;

        const run = (err) => {
            if (err) {
                return next(err);
            }
            if (index >= middlewares.length) {
                return next();
            }
            const middleware = middlewares[index++];
            try {
                middleware(req, res, run);
            } catch (error) {
                next(error);
            }
        };

        run();
    };
};

const middleware_one = (req, res, next) => {
    if (req.url === '/middleware-one') {
        res.end('This is middleware one');
    } else {
        next();
    }
};

const middleware_two = (req, res, next) => {
    if (req.url === '/middleware-two') {
        res.end('This is middleware two');
    } else {
        next();
    }
};

export default combineMiddleware(middleware_one, middleware_two);

Docs

Opened PR for updating docs in @astrojs/node integrations guide

withastro/docs#8789

Copy link

changeset-bot bot commented Jul 20, 2024

🦋 Changeset detected

Latest commit: c712fbe

The changes in this PR will be included in the next version bump.

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

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Jul 20, 2024
@nm123github nm123github force-pushed the feature/standalonemiddleware branch from 9a04a07 to 34ea7cc Compare July 21, 2024 00:28
@nm123github nm123github force-pushed the feature/standalonemiddleware branch 12 times, most recently from 2c48b81 to fdc526b Compare July 22, 2024 22:03
@nm123github nm123github force-pushed the feature/standalonemiddleware branch from 50001fc to 2e2829c Compare July 22, 2024 22:28
@nm123github nm123github force-pushed the feature/standalonemiddleware branch from 28c42ca to a0e5aa0 Compare July 23, 2024 01:53
@florian-lefebvre
Copy link
Member

I'm not sure I understand the usecase. Why can't you use middleware mode and pass your middleware to something like express?

@nm123github
Copy link
Author

nm123github commented Aug 26, 2024

Its mostly for use-cases where we could benefit from using publicly available middleware (e.g. http-proxy-middleware), and want to avoid overhead of adding ExpressJS.

@florian-lefebvre
Copy link
Member

We're not convinced by the usecase to be honest. We think middleware mode is exactly designed for what you want. You said you don't want the overhead of express but standalone mode is not meant to be a full fledge web server either. I'm sure you can find lighter alternatives than express if the bloat is what you're afraid of. Unless you have more arguments, I'm afraid we'll have to close the PR

@matthewp
Copy link
Contributor

Astro has middleware already, so adding a second way to add middleware that only works in SSR doesn't seem like a good choice for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants