Refactor routing functions to work without context

[nelsonprsousa]

Hi 👋 First of all thank you for your hard work towards this library. Awesome job! 👏

What I am bringing to discussion

I'd like to bring to discussion the refactor of routing functions, particularly translatePath (but others like localizePath could also benefit from it, even though I am not using it).

Current use cases where these functions do not work and could work

In the plugin.ts (or [email protected]) it could be valid to use translatePath, in order to perform redirects. For example, in my case, I do not have a "languageless" valid route. This means that example.com/ will redirect to, for example, example.com/en/ if en is the default language. Or, if the user changed the language to, let's say, fr, then we are saving a lang cookie to persist this preference. The goal it that when the user visits the site again through example.com/*, he will be redirected to example.com/cookiePreference/*.

AFAIK, and because these functions needs context, they will not properly work in the middleware.

Possible API

Based on my research on translatePath function, it is now using SpeakContext to get the rewriteRoutes and the current language. A possible API to remove the need of a context would be to pass the rewriteRoutes object and to make lang mandatory instead of optional.

export type TranslatePathFn = {
  /**
   * Translate a path
   * @param pathname The path to translate
   * @param lang The language to translate to
   * @returns The translation or the path if not found
   */
  (pathname: string, lang: string, rewriteRoutes: RewriteRouteOption[] | undefined): string;
  /**
   * Translate an url
   * @param url The url to translate
   * @param lang The language to translate to
   * @returns The translation or the url if not found
   */
  (url: URL, lang: string, rewriteRoutes: RewriteRouteOption[] | undefined): string;
  /**
   * Translate an array of paths
   * @param pathname The array of paths to translate
   * @param lang The language to translate to
   * @returns The translations or the paths if not found
   */
  (pathnames: string[], lang: string, rewriteRoutes: RewriteRouteOption[] | undefined): string[];
};

Function used on my project as a workaround

Please note that I removed some code that is necessary for qwik-speak library ecosystem but not for my current project.

Click me

import type { RewriteRouteOption } from 'qwik-speak';

type TranslatePathFn = {
  /**
   * Translate a path
   * @param pathname The path to translate
   * @param lang Optional language if different from the default one
   * @returns The translation or the path if not found
   */
  (
    pathname: string,
    lang: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ): string;
  /**
   * Translate an url
   * @param url The url to translate
   * @param lang Optional language if different from the default one
   * @returns The translation or the url if not found
   */
  (
    url: URL,
    lang: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ): string;
  /**
   * Translate an array of paths
   * @param pathname The array of paths to translate
   * @param lang Optional language if different from the default one
   * @returns The translations or the paths if not found
   */
  (
    pathnames: string[],
    lang: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ): string[];
};

export const translatePath = (): TranslatePathFn => {
  /**
   * To file-based routing
   */
  const normalizePath = (
    pathname: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ) => {
    // Source by prefix
    const source = rewriteRoutes?.find(
      (rewrite) =>
        pathname === `/${rewrite.prefix}` ||
        pathname.startsWith(`/${rewrite.prefix}/`) ||
        pathname.startsWith(`${rewrite.prefix}/`)
    );

    if (source) {
      pathname =
        pathname === `/${source.prefix}`
          ? '/'
          : pathname.startsWith('/')
            ? pathname.substring(`/${source.prefix}`.length)
            : pathname.substring(`${source.prefix}/`.length);
    }

    // In prod, source by current lang in domain-based routing
    // if (!isDev) {
    //   if (config.domainBasedRouting?.prefix === 'as-needed') {
    //     if (!source) {
    //       source = rewriteRoutes?.find(
    //         rewrite => rewrite.lang == currentLang
    //       );
    //     }
    //   }
    // }

    if (source) {
      const sourceEntries = Object.entries(source.paths).map(([from, to]) => [
        to,
        from,
      ]);
      const revertedPaths = Object.fromEntries(sourceEntries);

      const splitted = pathname.split('/');
      const translated = splitted.map((part) => revertedPaths[part] ?? part);
      return translated.join('/');
    }

    return pathname;
  };

  const rewritePath = (
    pathname: string,
    prefix: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ) => {
    let splitted = pathname.split('/');

    const destination = rewriteRoutes?.find(
      (rewrite) => rewrite.prefix === prefix
    );

    // Destination by prefix
    if (prefix && destination) {
      const keys = Object.keys(destination.paths);
      const translating = splitted.some((part) => keys.includes(part));

      if (pathname === '/' || translating) {
        pathname = pathname.startsWith('/')
          ? `/${prefix}` + pathname
          : `${prefix}/` + pathname;
      }
    }

    // In prod, destination by lang in domain-based routing
    // if (!isDev) {
    //   if (config.domainBasedRouting?.prefix === 'as-needed') {
    //     if (prefix && !destination) {
    //       destination = rewriteRoutes?.find(
    //         rewrite => rewrite.lang === prefix
    //       );
    //     }
    //   }
    // }

    splitted = pathname.split('/');
    const translated = splitted.map((part) => destination?.paths[part] ?? part);
    return translated.join('/');
  };

  const slashPath = (pathname: string, rewrote: string) => {
    if (pathname.endsWith('/') && !rewrote.endsWith('/')) {
      return rewrote + '/';
    } else if (
      !pathname.endsWith('/') &&
      rewrote.endsWith('/') &&
      rewrote !== '/'
    ) {
      return rewrote.substring(0, rewrote.length - 1);
    }

    return rewrote;
  };

  const translateOne = (
    pathname: string,
    lang: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ) => {
    const normalized = normalizePath(pathname, rewriteRoutes);
    const rewrote = rewritePath(normalized, lang, rewriteRoutes);
    return slashPath(pathname, rewrote);
  };

  const translate = (
    route: (string | URL) | string[],
    lang: string,
    rewriteRoutes: RewriteRouteOption[] | undefined
  ) => {
    // lang ??= currentLang;

    // if (!rewriteRoutes) {
    //   if (isDev) logWarn(`translatePath: rewriteRoutes not found`);
    //   return route;
    // }

    if (Array.isArray(route)) {
      return route.map((path) => translateOne(path, lang, rewriteRoutes));
    }

    if (typeof route === 'string') {
      return translateOne(route, lang, rewriteRoutes);
    }

    // Is URL
    const url = new URL(route);
    // if (config.domainBasedRouting) {
    //   url = translateDomain(url, lang);
    // }

    url.pathname = translateOne(url.pathname, lang, rewriteRoutes);

    return url.toString();
  };

  return translate as TranslatePathFn;
};

[email protected] if it also makes sense to add to this library to help others getting the same behaviour

Click me

import type { RequestHandler } from '@builder.io/qwik-city';
import { extractFromUrl } from 'qwik-speak';
import {
  AcceptLanguageHeaderName,
  LanguageCookieName,
} from '~/utils/constants';
import { inferPreferredLanguage } from '~/utils/inferPreferredLanguage';
import { setLanguageCookie } from '~/utils/setLanguageCookie';
import { config } from '../speak-config';
import { HttpStatusCode } from '~/models/httpStatusCode';
import { translateGoBeeGoPath } from '~/utils/translateGoBeeGoPath';
import { isValidLang } from '~/utils/inValidLang';

export const onRequest: RequestHandler = ({
  locale,
  url,
  request,
  cookie,
  error,
  redirect,
}) => {
  const getPath = translateGoBeeGoPath();

  const getLocalePath = (url: URL, lang: string): string => {
    const newUrl = new URL(url);
    newUrl.pathname = getPath(newUrl.pathname, lang, config.rewriteRoutes);

    return newUrl.toString();
  };

  let lang: string | undefined = undefined;

  const langFromUrl = extractFromUrl(url);

  if (langFromUrl && isValidLang(langFromUrl)) {
    lang = config.supportedLocales.find((value) => value.lang === langFromUrl)
      ?.lang;

    if (!lang) {
      throw error(404, 'Page not found'); // TODO: Custom 404 page
    }

    setLanguageCookie(cookie, lang, url.hostname);
    locale(lang);
    return;
  }

  const languageFromCookie = cookie.get(LanguageCookieName);

  if (languageFromCookie?.value && isValidLang(languageFromCookie.value)) {
    const nextUrl = getLocalePath(url, languageFromCookie.value);

    throw redirect(HttpStatusCode.FOUND, nextUrl);
  }

  const inferredPreferredLanguage = inferPreferredLanguage(
    request.headers.get(AcceptLanguageHeaderName)
  );

  if (inferredPreferredLanguage) {
    const nextUrl = getLocalePath(url, inferredPreferredLanguage);

    setLanguageCookie(cookie, inferredPreferredLanguage, url.hostname);

    throw redirect(HttpStatusCode.FOUND, nextUrl);
  }

  lang = config.defaultLocale.lang;

  setLanguageCookie(cookie, lang, url.hostname);
  locale(lang);
};

Let me know:

1. What do you think about the purposed refactor;
2. If it is worth exploring adding this rewrite + persist cookie language flow for others.

[robisim74]

Hi @nelsonprsousa,

Thanks for your detailed description.

Point 2. Thanks for sharing your solution. But I would avoid bringing it into the documentation: I think that for a library it is better to offer the tools, and then let each developer do their own implementation according to the requirements. Otherwise the documentation becomes difficult to maintain. Just as a side note, also keep in mind that redirects within an onRequest for the whole app are dangerous, because the onRequest is executed on every request to the server (not just when the page loads) and personally I think it's better to implement some sort of route guard for pages.

Point 1. The problem exists, you are right, but it exists only the first time the app starts. Then the shared context (such as the config) is saved on the server and made available for subsequent calls. The point is that onRequest runs before the app, so I'm currently undecided on which is the best solution:

• add the entire config (not just rewriteRoutes) as an optional parameter to the translatePath and localizePath methods, and explain that on the server both the lang and the config must always be passed (in order to avoid breaking changes)
• expose a new API, which like the QwikCity locale(lang) sets the config and makes it immediately available on the server for the whole app, and leave the translatePath and localizePath methods as they are

[nelsonprsousa]

Point 2. Totally get it. No worries. Would you please let me know what do you mean by route guards?

Point 1. Just letting you know that the first bullet is also a breaking change since lang is currently optional.

[robisim74]

By route guards I mean that the onRequest for redirects should be on every single page (or page group layouts).

The lang parameter becomes mandatory if the function is used server-side (as in inlineTranslate) because the server knows nothing about the current language: see https://github.com/robisim74/qwik-speak/blob/main/docs/translate.md#server-translation

[nelsonprsousa]

Makes sense, thanks for letting me know.

[nelsonprsousa]

Also, let me ask you if we can add the default lang to the rewriteRoutes? I added it and it seems to work, but I am unware if it could have some side-effects? The end goal, as in this discussion, is to always have the lang in the URL (with a redirect when we have no lang in the URL to the cookie or default one).

[robisim74]

Optional language in the URL for the default language is only a possibility (it is generally preferred to do this to avoid redirects for SEO and performance reasons), but it is not mandatory

[nelsonprsousa]

Got it. Actually I want to get rid of / as a whole exactly for SEO reasons and even human aware. In the end search engines will not index / but /en.

At least this strategy looks like the most wide spread recommendation across webmasters through a google search I did recently.

[robisim74]

Added setSpeakContext function to share the config server side. There is no need to refactor the functions.

See https://github.com/robisim74/qwik-speak/blob/main/docs/tutorial-routing-rewrite.md#routing ((with also indications of how to handle errors and redirects)

Greetings