QuiiBz / next-international

Type-safe internationalization (i18n) for Next.js
https://next-international.vercel.app
MIT License
1.19k stars 52 forks source link

[Middleware] Question: is the Next International middleware "really" chainable? #354

Closed gustaveWPM closed 4 months ago

gustaveWPM commented 4 months ago

Hello!

I played a bit with middleware this morning, and I would like to know if the Next International middleware is chainable.

For example, let's admit I would like to chain some rewrites:

// myRewrite.ts
import type { NextMiddleware, NextFetchEvent, NextRequest } from 'next/server';

import { NextResponse } from 'next/server';

const REWRITE_ME = '/rewrite-me/';

function getRewrittenPath(path: string) {
  if (path.endsWith(REWRITE_ME.slice(0, -1)) && path.length === REWRITE_ME.length - 1) {
    return '/';
  }
  return path.replace(REWRITE_ME, '/');
}

const doMyRewrite = (next: NextMiddleware) => {
  return async (request: NextRequest, _next: NextFetchEvent) => {
    console.log('Running middleware...');

    await next(request, _next); // * ... Calling the next chain element

    const nextUrl = request.nextUrl;
    const path = getRewrittenPath(nextUrl.pathname);
    nextUrl.pathname = path;

    const response = NextResponse.rewrite(nextUrl);
    console.log(response);
    return response;
  };
};

const myRewrite = (next: NextMiddleware) => doMyRewrite(next);

export default myRewrite;
// middlewareChains.ts
const MY_CHAIN: MiddlewareFactory[] = [myRewrite, withI18n];
export const myChain = stackMiddlewares(MY_CHAIN);

Could I also do this?

// middlewareChains.ts
const MY_CHAIN: MiddlewareFactory[] = [withI18n, myRewrite]; // * ... Calling Next International middleware first
export const myChain = stackMiddlewares(MY_CHAIN);

I think it is not possible, because my middleware is never executed when I try this. It looks like Next International's middleware is "breaking" the chain.

Maybe it could be refactored a bit like myRewrite.ts to be chainable if I'm right, using await next(request, _next);.

Read also: https://reacthustle.com/blog/how-to-chain-multiple-middleware-functions-in-nextjs (Using the stackMiddlewares function)

EDIT: maybe returning a callable would be a better way to solve this, as does the Next Auth middleware? Idk. Maybe I'm just misusing the Next International middleware here...

EDIT 2: sorry, I wasn't even really aware of what I had in my codebase anymore and I said something silly.

ruymon commented 4 months ago

@gustaveWPM Hey! I am facing the same issue... Did you manage to fix it?

gustaveWPM commented 4 months ago

@gustaveWPM Hey! I am facing the same issue... Could you let me know if you managed to fix it?

Hi there! No, I keep the Next International middleware at the end of my chain atm.

ruymon commented 4 months ago

@gustaveWPM Do you have a example for that? I am currently struggling to get it running...

gustaveWPM commented 4 months ago

@gustaveWPM Do you have a example for that? I am currently struggling to get it running...

Sure.

gustaveWPM commented 4 months ago

Also, having plunged my head back into this part of my code has made me realize that I'm wrong.

It's been a long time since I've touched this middleware chaining system, and in its current version, I have the impression that it's quite possible to do: withI18n(request)(request, _next) too.

Chaining is agnostic as to whether the middleware is "Designed for it" or not.

Closing this issue for now.

Btw, there's a caveat: https://github.com/QuiiBz/next-international/issues/341

ruymon commented 4 months ago

@gustaveWPM I've managed to rewrite the Middleware... It is simple and a bit restricted. But what matters is: it works.

Note is currently stuck to use the default urlMappingStrategy

// i18n-middleware.ts

import {
  NextResponse,
  type NextFetchEvent,
  type NextRequest
} from 'next/server';

import { CustomMiddleware } from './chain';

export type I18nMiddlewareConfig<Locales extends readonly string[]> = {
  locales: Locales;
  defaultLocale: Locales[number];
  urlMappingStrategy?: 'redirect' | 'rewrite' | 'rewriteDefault'; // Not implemented yet.
  resolveLocaleFromRequest?: (request: NextRequest) => Locales[number] | null;
};

export const LOCALE_COOKIE = 'Next-Locale';
export const LOCALE_HEADER = 'X-Next-Locale';

export const i18nMiddlewareConfig: I18nMiddlewareConfig<string[]> = {
  locales: ['en', 'pt'],
  defaultLocale: 'en'
}

export function withI18nMiddleware(middleware: CustomMiddleware) {
  return async (
    request: NextRequest,
    event: NextFetchEvent,
    response: NextResponse
  ) => {
    const locale = localeFromRequest(i18nMiddlewareConfig.locales, request) ?? i18nMiddlewareConfig.defaultLocale;
    const nextUrl = request.nextUrl;
    const pathname = request.nextUrl.pathname;

    if (noLocalePrefix(i18nMiddlewareConfig.locales, pathname)) {
      const redirectPath = `/${locale}${nextUrl.pathname}`
      const redirectUrl = new URL(redirectPath, request.url)
      return NextResponse.redirect(redirectUrl)
    }

    const pathnameLocale = nextUrl.pathname.split('/', 2)?.[1];

    if (!pathnameLocale || i18nMiddlewareConfig.locales.includes(pathnameLocale)) {
      response.headers.set(LOCALE_HEADER, locale);

      if (request.cookies.get(LOCALE_COOKIE)?.value !== locale) {
        response.cookies.set(LOCALE_COOKIE, locale, { sameSite: 'strict' });
      }
    }

    return middleware(request, event, response)
  }
}

function localeFromRequest(
  locales: string[],
  request: NextRequest,
  resolveLocaleFromRequest: (request: NextRequest) => string | null = defaultResolveLocaleFromRequest
) {
  const locale = request.cookies.get(LOCALE_COOKIE)?.value ?? resolveLocaleFromRequest(request);

  if (!locale || !locales.includes(locale)) return null;

  return locale;
}

const defaultResolveLocaleFromRequest: NonNullable<I18nMiddlewareConfig<any>['resolveLocaleFromRequest']> = request => {
  const header = request.headers.get('Accept-Language');
  const locale = header?.split(',', 1)?.[0]?.split('-', 1)?.[0];
  return locale ?? null;
};

function noLocalePrefix(locales: string[], pathname: string) {
  return locales.every(locale => {
    return !(pathname === `/${locale}` || pathname.startsWith(`/${locale}/`));
  });
}
gustaveWPM commented 4 months ago

@gustaveWPM I've managed to rewrite the Middleware... It is simple and a bit restricted. But what matters is: it works.

Note is currently stuck to use the default urlMappingStrategy

[...]

The idea of passing the next middleware to be executed as an argument is good. Tbh, I was just too silly to remember that what I already have in my codebase makes it possible to do this abstractly and without needing to rewrite middleware...

https://github.com/Tirraa/dashboard_rtm/blob/main/packages/shared-types/src/Next.ts#L24

Sorry /shrug