refactor: Remove Express dependency

[isaachinman]

Fixes #186.

Proof of concept, needs review and testing. Was surprised to find a complete lack of test coverage here, so let's be certain we're not introducing any regressions, and that all functionality is well tested.

[gurkerl83]

I quickly reviewed the merge request. I`m concerned about the integration of LanguageDetector in index.js. Sure index re-exports this file but uses LD internally. When you look at the typescript definitions, the Express API is used quite often in LD. A while ago, I forked this project and tried to migrate it to HTTP. I started the migration using the index.d.ts and replaced Express types with node types. The problem back then was that Express provides a way more sophisticated API on-top of HTTP, but how functionality like caching can be approached. Maybe it's not necessary at all. Anyway, thx for pushing this forward!

[isaachinman]

@gurkerl83 Thanks for the feedback - I had completely overlooked the typings (another thing which should be covered by tests).

The PR in its current state does work for basic functionality in all four examples, including the examples/basic dir where vanilla HTTP is used without Express at all.

However, the typings do make it clear that there's more work to be done here. There's sneaky usage of the Express API that VSCode hasn't caught. For example:

app[verb || 'get'].apply(app, routes.concat(callbacks));

I don't know much about functionality like addRoute and would need to rely on the advice of others as to how we should proceed.

It's also made me realise that we'll need to take a closer look at each of the files in the languageLookups dir, as some or all of them likely depend on the Express API (cookies, path, query).

I'm out of time on this for now but would very much appreciate a collaborative effort!

[gurkerl83]

@iamjochem I have continued on my local branch, today. Express provides a powerful middleware concept, which to my knowledge is not available in vanilla HTTP provided by Nodes http library. The current approach is to use the connect library, which provides a middleware concept for http. After working on this for some time, I switched to next-i18next to see how this library gets integrated. Is my understanding correct that next-i18next uses the i18next-express-middleware but also others for locale subpath work?

To me, it's quite confusing spreading middleware logic throughout several libraries. Another question is about custom servers, which, according to the Next developers, gets not supported in serverless deployment. I saw a lot of projects are deploying Express in their Next project. But what is the difference, implementing a server through Nodes http library is basically a server, too right. So the general question is - are sever deployments supported in a serverless development concept, or do we need to fetch resources like i18n files directly with Next?

[isaachinman]

Hi @gurkerl83, I assume you meant to tag me on that last comment. Apologies, it's been awhile.

After looking through this work again, I agree that a straight refactor to vanilla http is not as straightforward as one would hope.

I do think that using something like connect would be best. Would that allow us a drop-in replacement?

To answer your questions regarding next-i18next: we use i18next-express-middleware primarily for language detection, including caching via cookies/headers/etc. The locale subpath work happens separately, but is based on the language detected, of course.

custom servers, which, according to the Next developers, gets not supported in serverless deployment

This is mostly true for the moment. However, there are incoming changes to the NextJs core that will allow us to consume (req, res) http signatures.

So - removing the Express dependency from this middleware is inherently valuable and will indeed support serverless in the future.

[isaachinman]

Bit of an update: as next-i18next is a very popular consumer of this package, I have refactored the simple example in local dev to not rely on express:

const connect = require("connect")
const http = require("http")
const next = require("next")
const nextI18NextMiddleware = require("next-i18next/middleware").default

const nextI18next = require("./i18n")

const app = next({ dev: process.env.NODE_ENV !== "production" })
const handle = app.getRequestHandler();

(async () => {
  await app.prepare()
  const server = connect()

  nextI18NextMiddleware(nextI18next).forEach(middleware =>
    server.use(middleware)
  )

  server.use((req, res, next) =>
    req.method === "GET" ? handle(req, res) : next()
  )

  http.createServer(server).listen(3000)
})()

This results in the following error:

TypeError: res.set is not a function
node_modules/i18next-express-middleware/lib/index.js:68:11

If I simply change the two occurrences of res.set to res.setHeader in index.js, I seem to be able to fully run the server without express, just using connect and https.

@jamuhl What are your thoughts on changing the goal here a bit to be: "works either with Express or with Connect"? I think vanilla http support is not a realistic or even helpful goal. If you are supportive of this direction going forward, I can put forth a very simple PR to just remove those Express-specific res.set calls.

[jamuhl]

@isaachinman sure...for me that new goals if covering the needs in next-i18next are very welcome...👍

PR welcome

[isaachinman]

Opened #199. Going to close this PR now as the goal is no longer to fully remove Express, but rather to allow users to use the connect/http option as an alternative.