Use universal-middleware

[magne4000]

Notable changes

• Use @universal-middleware/compress for compression
• All cache related features have been dropped
• getPageContext hook is removed. The whole context from universal middlewares is given as a Context to renderPage. This means that the same effect can be achieved by any custom universal middleware.
• No more distinction between node and non-node envs (at least for now 🤞)

Next steps (not in this PR)

• Put cache feature back?
• Is it possible to have some generic way to handle HMR websocket connections, perhaps with crossws?

[jasonhilldm]

Hi. I stumbled across this extension while trying to figure out the best way to deploy to Node and it looks awesome. The one issue I have is that I am using Auth.js for authentication and was previously using universal middleware to add the logged in user to the context.

When I use the vike-node extension though, my modified context is not being seen by Vike so I assume it's using a different context. I'm assuming these draft changes will mean my context changes will be seen by Vike via the use of universal middleware...is that right?

Not sure how far away these changes are, but is there another way that I can add the user to the context using regular middleware so that Vike will see that when I am using vike-node?

[magne4000]

@jasonhilldm I would suggest to wait for this PR to be finished, as it will fix your issue.
Nevertheless, you could still retrieve the context for your server using the getContext function of your server inside the pageContext hook. Something like this should do the trick (untested, and probably does not work for all servers):

import { getContext } from "@universal-middleware/hono";

app.use(
  vike({
    pageContext: (req) => getContext(req)
  })
)

[jasonhilldm]

Thank you - that worked perfectly.

[brillout]

I started reviewing a bit, I'll resume tomorrow.

[magne4000]

Migration

• Added to BREAKING CHANGE commits

0.1.x to 0.2.x

Caching support removed

app.use(
  vike({
    compress: false,
-     static: {
-       cache: false
-     }
  })
)

pageContext

If you were using it to feed universal-middleware context to pageContext, it's now the default behaviour.

Otherwise, you now need to create a universal context middleware and attach it to your server.

app.use(
  vike({
-    pageContext: (req) => ({
-      user: req.user
-    })
  })
)

[brillout]

Add those as BREAKING CHANGES when squash merging

How about we push empty commits that include these BREAKING CHANGE: entries? It's a bit easier for the squasher.

[brillout]

We can force push if you want to edit previous BREAKING CHANGE: entries.

[magne4000]

BREAKING CHANGE commits updated

[magne4000]

Seems like we should be good to merge @brillout

[brillout]

After bundling and publishing this middleware as @universal-middleware-examples/tool, one can then use this middleware as follows:

I think it's even more misleading. Many users will wonder "why should I publish my middleware?".

(I guess the docs is automatically copy-pasting real code from examples to the docs. FYI I used to do that as well but I stopped doing it precisely because of things like this.)

[brillout]

A BREAKING CHANGE: is missing mentioning vike-node/connect.

Also:

- BREAKING CHANGE: `getPageContext` has been removed
+ BREAKING CHANGE: the `pageContext` setting has been removed

[brillout]

LGTM otherwise. I've pushed a couple of commits, let me know if you disagree with any of it.

I've mixed feelings about removing the cache. I kinda liked that the compression was being cached, but I ain't sure whether caching should be done inside vike-node (nor compression for that matter). Maybe we could have vike-server-compress and vike-server-cache extensions. Also a vike-plus that automatically adds all kinds of nice-to-haves.

As for this PR, I'm a bit inclined to remove the compression outside of vike-node. But I'm fine if we keep it for now. We'll see how we want to structure everything as we move forward.

[magne4000]

LGTM otherwise. I've pushed a couple of commits, let me know if you disagree with any of it.

LGTM

I've mixed feelings about removing the cache. I kinda liked that the compression was being cached, but I ain't sure whether caching should be done inside vike-node (nor compression for that matter). Maybe we could have vike-server-compress and vike-server-cache extensions. Also a vike-plus that automatically adds all kinds of nice-to-haves.

As for this PR, I'm a bit inclined to remove the compression outside of vike-node. But I'm fine if we keep it for now. We'll see how we want to structure everything as we move forward.

I think that the caching layer do not belong directly in this package. I not even sure that I find the feature useful (have we measured that it really improves perfs?).

Regarding the compression, no strong feeling about it. For the sake of merging this PR sooner rather than later, I'd say we keep it as-is for now.

[brillout]

A BREAKING CHANGE: is missing mentioning vike-node/connect.

Also:

- BREAKING CHANGE: `getPageContext` has been removed
+ BREAKING CHANGE: the `pageContext` setting has been removed

This seems to be still missing.

[brillout]

I not even sure that I find the feature useful (have we measured that it really improves perfs?).

I see, good point. Indeed computing a cache key isn't gratis either and bloating memory isn't necessarily a good default.

Regarding the compression, no strong feeling about it. For the sake of merging this PR sooner rather than later, I'd say we keep it as-is for now.

Makes sense 👍

After bundling and publishing this middleware as @universal-middleware-examples/tool, one can then use this middleware as follows:

I think it's even more misleading. Many users will wonder "why should I publish my middleware?".

(I guess the docs is automatically copy-pasting real code from examples to the docs. FYI I used to do that as well but I stopped doing it precisely because of things like this.)

WDYT?

[magne4000]

A BREAKING CHANGE: is missing mentioning vike-node/connect.
Also:

- BREAKING CHANGE: `getPageContext` has been removed
+ BREAKING CHANGE: the `pageContext` setting has been removed

This seems to be still missing.

Hum no, it's there 29128b5

[magne4000]

After bundling and publishing this middleware as @universal-middleware-examples/tool, one can then use this middleware as follows:

I think it's even more misleading. Many users will wonder "why should I publish my middleware?".
(I guess the docs is automatically copy-pasting real code from examples to the docs. FYI I used to do that as well but I stopped doing it precisely because of things like this.)

WDYT?

I'm currently working on some universal-middleware doc, I'll see what I can come up with.

[brillout]

I meant this one:

A BREAKING CHANGE: is missing mentioning vike-node/connect.

[magne4000]

2a4be82

[brillout]

Released as 0.2.0 🚀

[brillout]

I made a couple of improvements to the readme, see the last commits. Let me know if you disagree with anything.

[magne4000]

I made a couple of improvements to the readme, see the last commits. Let me know if you disagree with anything.

Thanks! LGTM