Server Extensibility

cristianbote · 11 February 2021 08:00

Hi @luisherranz, great IP! This will make Frontity amazingly extensible.

Order of middleware execution.
While reading through the details noticed that there is no guarantee of the order of middleware execution. Do you think this is important? Would this be something that folks would expect?
Escape hatch by exposing only the app.
While having a server property it’s pretty explicit and it makes a clear separation between what part of the app it’s involving, have you thought about having this kind of api? This practically will be called after the beforeSSR.

export default {
  server: {
    myPackage: ({ app }) => {

      // Sample of a middleware code
      app.use((ctx, next) => {
        const store = ctx.store;

        // At this point the `store` is available and it can do anything it wants with it.
      });
    },
  },
};

Allow interrupting the render flow.

Open Questions: We need to find a way to allow packages to “skip React SSR” so they can return other responses for URLs that should not be managed by React.

If we would allow the api at point #2 this will turn into simply handle the response on the server. Like this: https://codesandbox.io/s/bold-feistel-4og51?file=/index.js

Example:

export default {
  server: {
    myPackage: ({ app }) => {

      // Custom `/api` endpoint
      app.use((ctx, next) => {
        if (ctx.url.startsWith('/api')) {
          // Serialize the state, or other API responses, auth
          ctx.body = JSON.stringify(ctx.store.state, null, 2);
          return;
        }
        
        await next();
      });
    },
  },
};

If I am not mistaken @cristianbote is currently refactoring the core middleware. Cristian, could you please leave a message with the status of that refactoring? Thanks

Yup! I am done with it. The changes can be seen here: https://github.com/frontity/frontity/pull/687/files#diff-2aa5d096a9b58e6ba83f934e3f72bcff6ab4f27ca5cfe05dde74fe087ab53df5R93-R100.
At this point the store and settings are exposed on ctx.state but it’s not something that my implementation relies on so we can redefine those at will.

I am confident that allowing and exposing a function instead of a dictionary with middlewares, will empower the developers to extend at will the server as they wish.

What do you think @luisherranz? Did something slipped through my mind?

luisherranz · 12 February 2021 11:09

Our plans for this were to add priorities to packages. There is a small description on the AMP Implementation Proposal: AMP package - #10 by luisherranz

What I am wondering lately is at which level we should add priorities:

At a package level.
At a function level.

For some of the things, like Html2React processors and Source handlers, we have defined them at a function level. It’s more complex, much also more versatile.

WordPress also has them at a function level for actions (add_action) and filters (add_filter).

We should start a conversation about this

The code you mention will work because app is exposed in ctx.

However, I think our recommendation should be to use a normal Frontity middleware, as described in this IP, and await for next() when you need to run something after the SSR.

You are right. Most of the time, when the rendering should be skipped, it would be enough to not call next().

I wonder if there will be sometimes when skipping the React SSR AND keep executing the rest of the middleware (from other packages) will be necessary.

I guess it’s better to wait until we see more real usage of this

By the way, the API you describe is not required for this. "Not calling next()" is possible with the normal Frontity middleware functions described in this IP.

Awesome, thanks!

Yes, we will have to change that when we work in this implementation.

If you read my IP draft, that was one of the options explained there (section C). It included the pros and cons of not only that approach but also the alternatives: Server Extensibility - #13 by luisherranz.

It’s not too late to change some parts of the IP, so if you have a strong opinion about that specific part or any other, or you want to point out additional pros/cons of each approach that were not reflected in that draft, please do so

mmczaplinski · 19 February 2021 22:53

Awesome IP @luisherranz

I’m sure there’s a reason that I missed here, but why do we want to support both signatures?

By the way, good points @cristianbote I also read the Final Implementation first and didn’t realize that priorities were discussed elsewhere and exposing the app was in the draft IP

luisherranz · 22 February 2021 14:40

I have just realized there is another important reason to use namespaces that I forgot to add to the Proposal Implementation Draft (section B1): The DevTools.

If we use namespaces, we will have much better information for the server DevTools in the future, because after we run each middleware we can report it to the DevTools, the same way we will do with actions:

Run server.somePackage.some
- It mutated state.xxx.counter: 1 → 2
- It run actions.zzz.toggle(true).
  - It mutated state.zzz.other: false → true
Run server.otherPackage.some
- …

Because one is “the Frontity way” and the other is needed to reuse Koa middleware. It is explained in the draft, section D3.

import ratelimit from "koa-ratelimit";

export default {
  // ...
  server: {
    myPackage: {
      myMiddleware: ({ ctx, state }) => {
        // Do stuff with ctx, state and so on...
      },
      rateLimiter: ratelimit({
        // Add koa packages directly.
      }),
    },
  },
};

cristianbote · 22 February 2021 21:09

Ok, took some more time to read through the IP and I am confident that I don’t see my proposal . Maybe I didn’t lay it properly. Let me reiterate over it again, in a more structured way.

D4 Scoped app handler

In a middleware based, server framework, the order of the middleware matters. @luisherranz has talked about it more in this Onboarding Session Video. So, in order to let the server be extensible by using a package, we can define a scoped app handler per package:

export default {
  // ...rest of the configuration
  server: {
    myPackage: ({ ctx, app }) => {
      // Configure the app, middleware and such.
      app.silent = true;
      app.on("error", () => {
        // Do something...
      });

      // Package specific middleware
      app.use(/* middleware fn foo */);

      // Package specific middleware
      app.use(get('/api/cart', /* ... */));
    },
  },
};

For a safer multi package execution, each scoped server handler could be called not with the main app instance but with a new one, that would be mounted. A sub-app per se.

Object.values(store.server).forEach(handler => {
  const app = new Koa();
  handler({ ctx, app});

  // Mount the app.
  rootApp.mount(app);
});

Pros:

Stays true to the Koa pattern and paradigm. It is after all direct access to app instance.
Direct access to an app instance could lead to more experimentations(React Server Components with their custom api, static serve of resources, Etag with a in-memory cache layer, etc).
The order of the middleware is defined and owned by the package.
Easier mounting of existing node Koa server. They could use their existing server with the predefined middleware, by mounting their previous one:

import previousServer from './legacy/server';

export default {
  // ...rest of the configuration
  server: {
    myPackage: ({ ctx, app }) => {
      // Mount an existing server
      app.use(mount(previousServer));
    },
  },
};

Cons:

I believe this is the first deviation of how frontity has previously defined the scoped settings for a given namespace, so needs to be handled and explained in a such manner.

At the end of the day, the most important aspect is to let the server be extendable. I do not hold any variant close to heart. I see them as different ways of achieving the extensibility we desire.

I do lean a bit more on the above D4 as I can see more usages out of it.

luisherranz · 2 March 2021 12:37

Thanks for the explanation Cris!

Absolutely! That’s the main goal I think.

This is a really good point. I didn’t include anything about priorities in this IP, but I think we should start taking a deeper look at them.

The problem that I see with D4, or similar approaches, is that they solve the middleware prioritization (order of execution) problem only for the middleware of a given package. It doesn’t solve the problem between packages. It also does so at the expense of obscuring/isolating the middleware functions of a package from the rest of the application, which is something I would prefer to avoid.

In the AMP Implementation Proposal I added a “Package Priorities” feature which would give packages the opportunity to set a package-wide priority in their frontity.config.js file. I don’t think that that is going to be flexible enough either. We need to find a way to set more fine-grained priorities that work across packages.

So we should find a way to handle priorities in Frontity so that:

They can be used at a function level.
They work consistently across all the different APIs.
They work across packages.
They can be easily configured by package creators.
They can be easily modified by other packages if needed.
Ideally, they can be easily modified by final users if needed.

The Frontity APIs that need priorities right now are:

Source Handlers.
Html2React Processors.
Fills (Slot and Fill).

Other APIs that are going to need priorities soon are:

Server Middleware Functions.
Packages themselves (the order in which they are merged together).
Frontity Hooks/Filters (this FD).
Custom configurations of frontity.config.js:
- Webpack modifications.
- Babel modifications.
- Site Settings modifications.

I am sure there will be more in the future.

I am going to open a separate Feature Discussion for “Frontity Priorities” so we can start brainstorming some ideas there

EDIT: I have opened the Feature Discussion Frontity Priorities

cristianbote · 3 March 2021 08:04

Are you referring here that a package should be able to change/modify/remove another’s package server middleware?
If that’s the case, I can’t think of an end-result where a package would modify another package middleware. Do you have an example of a use case in mind? Or is this related to something like:

// `foo` package
{
  server: {
    foo({ app }) {
      app.use(mount('/api/state', async (ctx, next) => {
        ctx.body = JSON.stringify({ foo: true });
        // not calling `await next();` here stops the whole execution
      }));
    }
  }
}

// `baz` package
{
  server: {
    baz({ app }) {
      app.use(mount('/api/state', async (ctx, next) => {
        ctx.body = JSON.stringify({ baz: true });
        // not calling `await next();` here stops the whole execution
      }));
    }
  }
}

// What would this endpoint return?
-> http://localhost:3000/api/state

By “rest of the application” you mean the root Koa app instance? As I look over my proposal that’s the only part that would not be able to be changed directly, which can be a good thing, but rather through its own isolated, nested app.

I’ve recorded a short video about it as well.

luisherranz · 4 March 2021 11:20

Understood. Thank you a lot for the video Cris

I’ve made another video with my impressions:

As I say in it, I think your proposal makes a lot of sense, although I still think the other one is slightly more flexible in terms of extensibility.

Let me know what you think!

nicholasio.oliveira · 4 March 2021 16:47

@luisherranz I spent some time reviewing the IP and I think it makes a lot of sense, this will make Frontity extremely flexible.

I’m still going through some of the back and forth on this topic but I’m planning on starting helping to implement this in the next few days.

cristianbote · 4 March 2021 19:45

Totally make sense @luisherranz. Thanks for taking the time to explain it.

It’s much clear now what we try to achieve, even though an applied use-case of that middleware prioritisation, it’s a bit difficult to visualise in a proper context.

Ok, I think we can conclude this. Thank you for listening to my comments.

luisherranz · 5 March 2021 08:31

On the contrary, thanks to you for taking a look from a new perspective at this

That’s awesome @nicholasio.oliveira! Let us know if you need any help or have any questions.

By the way, just as a reminder for people reading this, this is the Feature Board where we are going to keep track of our progress: Server Extensibility · GitHub

mmczaplinski · 19 March 2021 00:45

I wanted to share some thoughts about passing the ctx to the server actions like beforeSSR() afterSSR() and init().

luisherranz · 19 March 2021 15:05

It’s a good point, Michal.

If we follow our design principle “We should not create more than one API for the same purpose” it is clear to me that we should deprecate context in the actions and suggest people to use only middleware. It also has the benefits of prioritization and so on.

I didn’t remove it because I thought that for some use cases the code of beforeSSR/afterSSR would be simpler than the code of the middleware.

I wrote an example in this section (you can click on the arrow and it should scroll):

But to be honest, I think Michal is right. This slight improvement doesn’t justify the confusion and complexity of having two ways to access the server context.

What do you think guys?

luisherranz · 19 March 2021 15:09

Actually, we could have a state property that guarantees that this request has been SSR’ed by Frontity. Fro example:

export default {
  server: {
    myPackage: {
      afterReactSSR: async ({ ctx, state, next }) => {
        await next();
        if (state.frontity.rendering === "ssr") {
          // ...
        }
      },
    },
  },
};

fancheng33 · 25 September 2021 17:01

Hi @luisherranz great IP. Are there any progress on the implementation of this feature? It would lower the adoption friction of Frontity in large organizations where we do need to integrate with the middleware-based Node.js Koa ecosystem for common usage scenarios.

Thanks for all the great work!

luisherranz · 28 September 2021 07:40

There’s only an unfinished PR from @David: https://github.com/frontity/frontity/projects/4

If you want to contribute, please go ahead

mike1 · 14 December 2021 15:23

New here so trying to get my head around architecture. Is there a way to apply a koa middleware package? I’d like to use koa-passport.

luisherranz · 1 February 2022 10:39

Take a look at the existing solution of this post: Server Extensibility - #13 by luisherranz

orballo · 11 May 2022 12:10

Just FYI an update on this feature. It’s being developed on this PR Feature/server extensibility by orballo · Pull Request #916 · frontity/frontity · GitHub

luisherranz · 19 May 2022 15:17

I’m about to merge @orballo’s PR

It implements the server extensibility as described in the Final Implementation proposal, so there’s no need for any correction.