301 redirects stored in WordPress database

Description

There are some WordPress plugins that store the redirects in the database. As Frontity is accessing the REST API, it’s not aware of these redirects, so they could stop working if users are not using the Embedded mode. We should find a way to support it for all those users migrating to Frontity who want to keep the redirects.

Possible solution

We could add some logic to tiny-router to fetch the urls in parallel to getting the content from the REST API, and modify the Frontity side if the fetch return a 301 in the Header. This would mean that is doing two different requests for the same url. It could include a setting to specify this behaviour:

  • none: the default behaviour, where Frontity doesn’t do the new fetch.
  • 404: it just does the fetch if it’s a 404 error.
  • all: it does the fetch for all the urls.
  • regex: allow users to filter using regex.

Implementation proposal

After a lot of deliberation and trial and error here we are :slight_smile:

You can refer to the pull request which contains all the details.

1. router state

I propose the following state and the naming:

/**
 * How the router should handle 301 redirections that can be stored in the
 * WordPress database e.g. via a 
 * Redirection plugin: https://wordpress.org/plugins/redirection/.
 *
 * - "none" - Does not handle them at all.
 *
 * - "all"  - Always make an additional request to the WordPress instance
 * to check if there exists a redirection.
 *
 * - "error" - Only send the additional request to the WordPress instance
 * if the original request returned a 404. This is the default.
 *
 * - Regexp  - Make the additional request for links that match the given regex.
 *
 * @defaultValue "none"
 */
redirections: "none" | "all" | "error" | RegExp;

My only doubt here is if we should even provide the option to follow "all" redirections. I think most of the time the users should just use the "error" default and the "all" setting would not make sense for them.

I feel that the risk here is that users might assume that they should use the "all" setting when in reality, they will only need the "error" setting and their performance will suffer without them even noticing because they will make a useless fetch(link, { method: 'HEAD' }) for every link.

2. Modification to source.actions.fetch()

We’ll need something very similar to the following inside of the error handler:

if (e instanceof ServerError) {
  if (state.router.redirections === "error") {
    // TODO: handle errors
    const head = await fetch(state.source.url + link, {
      method: "HEAD",
    });

    if (head.redirected) {
      // TODO: We'll have to preserve the query string as well.
      // TODO: I guess we should also normalize the link with libraries.source.normalize(link);
      const newLink = new URL(head.url).pathname;

      Object.assign(source.data[link], {
        location: newLink,
        is301: true,
        isFetching: true,
        isReady: true,
        isRedirection: true,
      });
      return;
    }
  }
}

3. Should we implement state.source.url together with this feature ?

This feature would be quite a lot cleaner if we could already use the state.source.url.. Otherwise, we would have to parse the link ourselves to get the full backend url.

4. Router actions.

We ll need three modifications in the router actions:

4.1 actions.router.init():

This is needed because we want to “react” whenever the data for the redirection has been fetched on the client and call actions.router.set() with the correct URL again.

observe(() => {
    const data = state.source.get(state.router.link) as RedirectionData;
    if (data.isRedirection && state.frontity.platform === "client") {
      actions.router.set(data.location, { method: "replace" });
    }
  });

4.2 actions.router.beforeSSR()

Handle the redirection on the server side if the user is loading the redirected page directly.

const data = state.source.get(state.router.link) as RedirectionData & ErrorData;
if (data.isRedirection && state.router.redirections === "error") {
  // TODO: Handle all of the the Frontity Options, not just `frontity_name`
  ctx.redirect(
    data.location + "?" + `frontity_name=` + state.frontity.options.name
  );
} else if (data.isError) {

4.3 Change actions.router.set() to use the link from the redirection, if the data for the redirection has already been fetched:

const data = state.source.get(link) as RedirectionData;
if (data.isRedirection) {
  link = data.location;
}

Finally, it’s worth mentioning that the redirections for both RegeEx and all should work analogically, but are omitted here for clarity.

2 Likes

Great work Michal!

This is my feedback:

I think we should! :slightly_smiling_face:

1. Use "no" instead of "none"

To be consistent with the "no" we use for state.theme.autoPrefetch.

export type AutoPrefetch = "no" | "in-view" | "hover" | "all";

2. Default to "error" or "no"?

I’m not sure if the default should be error or no. My current thought is that the default should be no.

3. Use "404" for 404’s and "error" for errors

In Frontity 404’s are only a subset of errors. In order to be consistent, I would call this option "404" instead of "error". If we also think that a setting for all errors is worth (which I’m not sure right now) then I would call it "error".

  • "error": data.isError === true
  • "404": data.is404 === true

4. Make state.router.redirections an array of settings

I agree with you that RegExp and 404’s need to be used at the same time so we should allow for an array of settings:

  • state.router.redirections: "404"
  • state.router.redirections: "\\/some\\/url"
  • state.router.redirections: ["404", "\\/some\\/url"]

It would also be useful if people need to add more than one RegExp:

  • state.router.redirections: [ "\\/some\\/url", "\\/some\\/other\\/url"]

5. Use the same pattern/regexp strings as in handlers

We could also allow the same format of patterns and RegExps that we are using in handlers, because patterns are much simpler for certain URLs, although not as powerful as regular expressions.

  • "/some/custom/:urls" (pattern).
  • "RegExp:\\/some\\/custom\//(?<urls>[^/$?#]*)" (RegExp).

6. Don’t follow redirects

In order to avoid unnecessary bandwidth consumption, the fetch we do to check for a 301 should not follow redirections:

const head = await fetch(state.source.url + link, {
  method: "HEAD",
  redirect: "manual",
});

7. Support both 301 and 302

I would try to be consistent with how we define 400-500 errors:

const error = {
  isError: true,
  is500: true,
  errorStatus: 500,
  errorStatusText: "Some string describing the error",
};

And also add support for 302 redirections:

const redirection = {
  isRedirection: true,
  is302: true,
  location: newLink,
  redirectionStatus: 302,
};

Also location is the name of the 301 header, but maybe it would make more semantic sense to use a different field in Frontity, like newLink or redirectionLink?

There a couple of extra redirections that use 307 and 308, but to be honest I’ve never seen them used.

8. Bypass Server Side Rendering if there’s a redirection

For what I read in the Koa docs, it seems like ctx.redirect() only populates the status and location headers, but the app keeps executing:

ctx.status = 301;
ctx.redirect("/cart");
ctx.body = "Redirecting to shopping cart";

https://koajs.com/#response-redirect-url-alt

So I think the framework should be smart enough to bypass the React server-side render if the ctx.status is either 301 or 302 to avoid unnecessary work.


Finally, I’d like to propose another related feature that may be worth thinking about at this point: Frontity redirections (as opposed to “Backend Redirections”). I’ll do so in another post later in the day.

I don’t have time today. I’ll try to do it tomorrow.

Basically, I was thinking that maybe we should let room for configuring 301/302 redirections directly in Frontity, instead of in WordPress, and support both.

const state = {
  router: {
    redirections: {
      frontity: {
        "/some-post": "/other-post",
        "/some/custom/:url": "/other/custom/${params.url}",
        "/more/custom/:url": {
          destination: "/other/custom/${params.url}",
          status: 302,
        },
      },
      source: ["404", "/redirections/stored/in/wordpress"],
    },
  },
};

Maybe not for now, but if we want to add this in the future, we have to take that into account so the API names make sense in combination of WordPress/backend redirections.