Scully: [SEO] - Add trailing slash to generated href's

[michielkikkert]

🧩 Feature request

[SEO] - Add trailing slash to generated href's

Description

Currently, the href's in the statically generated HTML do not have a trailing slash. This is decremental for SEO as the generated static site structure (once 'hosted') does need this trailing slash to request the file.

So currently an href in the HTML looks like:
\Business Hosting

This means that when the Search Engine spider comes along, it will want to index/follow "/hosting/business-hosting" (without trailing slash). However, if this path is requested from the 'server' - it expects a trailing slash as it really is a folder (with a default file: index.html). You can see this happening when you refresh any page on the scully server even. It will do a 301 redirect to the version WITH slash. This is - especially for SEO purposes, but also for performance (additional unneeded server roundtrip) - not ideal.

Describe the solution you'd like

Provide an option to automatically add a trailing slash to any (Scully generated) href route in the generated HTML.

Describe alternatives you've considered

No easy alternatives - besides maybe doing this manually in a cloud worker.

[msacket]

I have a couple of ideas on this.
1) Write a render plugin that performs the update (could probably do it with the regex one)
2) Override the RouterLinkWithHref directive with something like this in app.module.ts:

import { RouterLinkWithHref } from '@angular/router';

RouterLinkWithHref.prototype.ngOnChanges = function() {
  const url = this.locationStrategy.prepareExternalUrl(this.router.serializeUrl(this.urlTree));
  this.href = `${url}/`;
};

Doing it that way... appears to prevent any router side affects.

Here's a StackBlitz: https://stackblitz.com/edit/angular-sr467n

[michielkikkert]

Good suggestions - although I would like to steer away from having to do this in Angular itself (as it's really only applicable to the statically rendered Scully site). I'd like to keep my Angular sites as clean as possible (preventing Universal nightmarish things). I think a plugin would be fairly good to do as well. I think @SanderElias is already thinking about where this belongs exactly (might be a core thing with a config flag in scully.config - not sure).

[msacket]

@MikeOne I agree. Idea 2 is fairly hackish and subject to breakage. A config flag would be better. I first wondered about hooking into or supplanting the LocationStrategy, but I couldn't find a way to do that in with a public API.

[michielkikkert]

Let's see what @SanderElias comes up with. If needed I can do the plugin for it if that ends up to be the correct way of doing it. I just made my first plugin today (add a canonical meta tag to all pages) - and it's pretty easy to do ;-)

[aaronfrost]

I think that this would be a VERY SIMPLE render plugin. It would find any pattern `href="something" (that doesn't end in a slash) and replace it with a slash before it saves it to file. It would be easy to write, and would execute quickly, I think.

There are some difficulties. Making sure that you not modifying an href that is inside of a string that's in some JS somewhere. You would need to make sure you weren't inside a script tag. But with JSDOM, this should be easy to avoid.

[aaronfrost]

const anchors = dom.document.body.querySelectorAll('a[href]');
Array.prototype.forEach.call(anchors, (a) => {
  const href =  a.getAttribute('href');
  const endsWithSlash = href.endsWith('/');
   if(!endsWithSlack) {
    a.setAttribute('href', href +'/';
  }
})

[msacket]

Good idea @aaronfrost. What about selecting on a[ng-reflect-router-link] to avoid modifying any non-Angular links? Is ng-reflect-router-link always present?

[michielkikkert]

Fairly simple indeed - however, as an additional sanity check, it might be wise to ONLY rewrite links that match with a generated Scully route.

[msacket]

Turns out ng-reflect-router-link is not always available. I think it's only available with a non-production build. Even routerlink isn't always there. Here are two examples:

<a href="" routerLink="/a">Link A</a>
<!-- renders via scully as -->
<a _ngcontent-rmj-c5="" href="/a" routerlink="/a">Link A</a>

<a href="" [routerLink]="['/b']">Link B</a>
<!-- renders via scully as -->
<a _ngcontent-rmj-c5="" href="/b">Link B</a>

[msacket]

Using the above, here's a plugin that I believe will work for most cases. I can submit a PR if the Scully team wants to include it.

const {registerPlugin, logWarn, yellow} = require('../dist/scully');
const jsdom = require('jsdom');
const {JSDOM} = jsdom;

const addTrailingSlashPlugin = async (html, options) => {
  try {
    const dom = new JSDOM(html);
    const {window} = dom;
    const anchors = window.document.querySelectorAll('a[href^="/"]');
    Array.prototype.forEach.call(anchors, (a) => {
        const href =  a.getAttribute('href');
        const endsWithSlash = href.endsWith('/');
        if ( !endsWithSlash ) {
            a.setAttribute('href', href + '/');
        }
    });
    return dom.serialize();
  } catch (e) {
    logWarn(`error in trailingSlash, didn't parse for route "${yellow(route.route)}"`);
  }
  return html;
};

const validator = async config => [];
registerPlugin('render', 'addTrailingSlash', addTrailingSlashPlugin, validator);

[michielkikkert]

Cool - just tried it, works fine (as expected). I would prefer though to add a check per href and see if the one you're changing is actually a scully rendered route (to prevent potentially breaking things in more complex sites). I understand that the full list of rendered route is not yet available for the render plugin (or any plugin I guess) - So I'll do a feature request for that. Maybe once that list is available we could adapt your plugin to use it?

[SanderElias]

@msacket Cool, I'll take this as the base I'll add the list of routes from #374 into the mix too.
We don't want to modify routes we don't "own"