Updating to @metalsmith/permalinks@2.5.1 breaks the build

[willeastcott]

Current version used is 2.4.1. Upgrade to 2.5.1 results in:

[willeastcott]

I spent quite some time trying to figure out what's wrong here. @epreston - if you have a spare moment, I was wondering if you had any thoughts on this.

[epreston]

I'll have a go.

[epreston]

Upgrading @metalsmith/permalinks from 2.4.1 to 2.5.1 the path property is not added to the file’s metadata (they like to use the term "front-matter" from book publishing). Some of the commits mention switching to a permalinks property.

It results in this function of build.mjs receiving a null or undedied value. I have modified it to show the issue:

// Convert relativeURL with a locale like en/manual to a full url with the
// desired locale e.g. https://developer.playcanvas.com/ja/user-manual
handlebars.registerHelper('locale-url', (locale, relativeUrl) => {
    if (!relativeUrl) {
        console.warn(`build.mjs : undefined relativeUrl passed to helper.`);
        return '';
    }

    relativeUrl = path.join(locale, relativeUrl.substring(2));
    const url = new URL(relativeUrl, 'https://developer.playcanvas.com/');
    return url.href;
});

The handlebars helper above is used in one file, head.hbs, for language switching.

The modification above allows the build to succeed but the links are obviously broken. I'm isolating the regression and raising an issue on the @metalsmith/permalinks project.

This should still work according to the documentation. See 3rd paragraph here: https://metalsmith.io/docs/getting-started/#the-plugin-chain

I'm still a little cloudy on this but, this project is using the following config:

    .use(permalinks({
        pattern: ':filename'
    }))

I don't know if :filename was ever a valid property unless it was set in every markdown file. The permalinks plugin had a fall-back behaviour when the pattern was not set (or invalid) that set the path property anyway. Now it seems its not setting path or the new permalink property that should replace it.

I'll update again when I learn more.

[willeastcott]

Wow, that's fantastic detective work @epreston! You've already surpassed my investigation. 🙏

[epreston]

You know me, I'm just doing it for attention.

[webketje]

After successfully verifying assertions in unit tests I cloned this repo and ran the build. I added a plugin after the permalinks plugin:

    // en/error/index.html was alphabetically first file to be rendered
    .use(files => console.log({
      path: files['en/error/index.html'].path,
      permalink: files['en/error/index.html'].permalink
    }))

This showed me that both the path & permalink property were present. So I had a look at your custom plugins to see if nothing 'funky' was being done there (as used to be the only solution when metalsmith had less out of the box).

There's a small docs section about metalsmith plugin order: https://metalsmith.io/docs/usage-guide/#plugin-order

Generally, you want plugins that inject new files or add metadata to be run at the start of the plugin chain so it is available in layouts and for other plugins to process

The tutorials() custom plugin generates files after permalinks has run and so it never sets permalink or path for all those files.

So really the minimal solution with permalinks 2.5.x is to move it after tutorials:

--- a/build.mjs
+++ b/build.mjs
@@ -132,6 +132,7 @@ Metalsmith(__dirname)
         renderer: renderer
     }))
     .use(contents())
+    .use(tutorials('tutorials')())
     .use(permalinks({
         pattern: ':filename'
     }))
@@ -157,7 +158,6 @@ Metalsmith(__dirname)
         contentPath: 'content/_shadereditor_contents.json',
         partialName: 'shader-editor-navigation'
     }))
-    .use(tutorials('tutorials')())
     .use(layouts({
         pattern: '**/*.html'
     }))

But would change the locale-url helper calls to use permalink instead of path.

That said, after screening this repo I think the build could use some updating-love. Things like metalsmith.ignore'ing .DS_Store files instead of doing build-time plugin checks, using metalsmith.env() to benefit from better defaults & debugging, but most of all these blocking fetch calls in the tutorials while loop and the synchronous file system calls.

The whole tutorials plugin could be made much simpler with a separate metalsmith pipeline using @metalsmith/requests and allowing to cache the remote-fetched content instead of fetching it on every rebuild, but

[epreston]

Thank you for taking a deep look at this @webketje

Completing the changes above, it works on 2.5.1 with permalink, path is not defined. For our purposes, this solves the issue cleanly. Heads up if you expect path to work.

I'll prepare PR's to incorporate your suggestions:

• metalsmith.ignore() for filtering
• metalsmith.env() for defaults and graceful debugging
• replace custom plugin with @metalsmith/requests in another pipeline

[webketje]

@epreston path definitely works for all files except those added by the tutorials plugin. Removing the line in the tutorials plugin that sets the path to dynamically added files makes it work. I haven't figured out exactly but in any case a log at @metalsmith/layouts execution shows that the file has both a permalink & path property at render time:

en/tutorials/360-lookaround-camera/index.html {
  title: '360 lookaround camera',
  layout: 'tutorial-page.hbs',
  tags: [ 'input', 'camera', 'tutorial', '_demo' ],
  mode: '0644',
  path: [Getter/Setter],
  locale: 'en',
  en: true,
  description: 'Sample showing how to move the camera with mouse and touch to look around',
  contents: <Buffer 3c 64 69 76 3e 53 61 6d 70 6c 65 20 73 68 6f 77 69 6e 67 20 68 6f 77 20 74 6f 20 6d 6f 76 65 20 74 68 65 20 63 61 6d 65 72 61 20 77 69 74 68 20 6d 6f ... 481 more bytes>,
  permalink: 'en/tutorials/360-lookaround-camera'
}

[epreston]

@webketje you're right. Thank you. I'm looking deeper at that now.

The custom tutorials plugin is giving us other issues with that path too when we adjust the order. I'm weighing the effort of going custom pipeline as you suggested vs patching this.

[epreston]

Right now, it looks golden except tutorials/index.html is being generated with links that contain an extra .html at the end. It should just point to the sub directory.

I think its isolated just to that custom plugin so I'm walking through it now.

[webketje]

I'm weighing the effort of going custom pipeline as you suggested vs patching this.

Custom pipeline has other advantages, such as being able to build offline, track changes to those files, and speed up repeat builds considerably. BTW since 2.6.0 Metalsmith has its own watch method which can do partial rebuilds (=even faster)

[epreston]

Sounds like I have some reading and work to do.

Fixed the issue above, one more patch to do then I will refactor to the best practice you have provided.