feat(gatsby-remark-copy-linked-files): Add absolutePath to dir function

[karlhorky]

Supersedes #27126 (see #27126 (comment))

Ref: #27097
Ref: #32735

Description

Draft of a fix for #27097, to pass the linkNode.relativeDirectory to the destinationDir callback.

Documentation

If this is an ok solution, then I can add documentation!

Related Issues

Closes #27097

[LekoArts]

I tried out things locally a bit and I'm not sure how relativeDirectory would work with this.

Can you give an example of a setup (e.g. with gatsby-config and some paths to files) and what the expected outcome is? For me, relativeDirectory is always an empty string because there is nothing relative

[karlhorky]

Below is my original motivating example config from #27097 .

It worked back then, but not sure if it still works now:

// gatsby-config.js
plugins: [
  {
    resolve: `gatsby-transformer-remark`,
    options: {
      plugins: [
        {
          resolve: "gatsby-remark-copy-linked-files",
          options: {
            destinationDir: f => `${f.relativeDirectory}/${f.name}`,
          },
        },
      ],
    },
  },
]

This was so that the output files would stay in the same folder structure that the original file is in, instead of losing the path in the built version.

Basically, a way to make https://github.com/akabekobeko/npm-gatsby-remark-copy-relative-linked-files obsolete.

[LekoArts]

Yeah, but what was your MD files setup? I tried it like this:

docs/nested-dir/index.md
docs/nested-dir/icon.png

gatsby-source-fileystem pointing to docs/nested-dir. The MD file contained ![image](icon.png). And the relativePath for that icon.png file node was just '' because

https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-filesystem/src/create-file-node.js#L20-L22

pathToFile and pluginOptions.path are already both in the same dir

[karlhorky]

Right, they were in the same directory - seem to remember it working.

Maybe something changed in gatsby-source-filesystem? Is there a "root-relative directory" that could be passed instead? I guess that's what I'm looking for.

[LekoArts]

I mean, if I'd point gatsby-source-filesystem only to docs and not docs/nested-dir I'd get nested-dir for relativeDirectory. But this seems a bit brittle.

I'm assuming you want a relativePathToSiteRoot? So we wouldn't rely on the relativeDirectory of the File node but just construct that path ourselves with process.cwd() and the full path to the file.

In theory you could already achieve this on your own at the moment by the way, couldn't you? You get f.path which you can use with __dirname and then construct that path on your own

[karlhorky]

I'm assuming you want a relativePathToSiteRoot? So we wouldn't rely on the relativeDirectory of the File node but just construct that path ourselves with process.cwd() and the full path to the file.

Oh ok, so this PR is more complicated than I thought. Not sure why I thought it was working before, maybe I didn't test it after all...?

If so, then would you recommend that I write a new function to calculate this relative path in the gatsby-remark-copy-linked-files/src/index.js file? I guess we could just call it path, since f.path doesn't yet exist (see below).

Maybe getNode(markdownNode.parent).dir or linkNode.absolutePath could be interesting somehow... just need to make these relative to the root Markdown directory (eg. docs in your example), if that would be available in a variable somewhere... 🤔

In theory you could already achieve this on your own at the moment by the way, couldn't you? You get f.path which you can use with __dirname and then construct that path on your own

f.path was just part of my original proposal - it is not in the gatsby-remark-copy-linked-files options that are available right now:

[karlhorky]

Hey @LekoArts, any thoughts on the above?

[LekoArts]

I was on vacation :)

Ok, so how about doing this:

• Expose the absolutePath
• Then a user can use f.absolutePath to construct their desired path

So in your case you'd use path utils together with __dirname to get your relative dir

[karlhorky]

Hope your vacation was great!

I'd be up for giving that a shot, sure! Is 95a8fcf what you mean?

If this is the way forward, I can give this a go in our application to see if I can use f.absolutePath + path + __dirname to achieve the same thing as https://github.com/akabekobeko/npm-gatsby-remark-copy-relative-linked-files

[LekoArts]

@karlhorky yeah, I'd say copy the things into your node_modules and give it a try. I pushed some further changes and a test

[karlhorky]

Cool, trying this now. Here's my patch-package patch, in case this is helpful for anyone else:

diff --git a/node_modules/gatsby-remark-copy-linked-files/index.js b/node_modules/gatsby-remark-copy-linked-files/index.js
index d37ee93..385ff4f 100644
--- a/node_modules/gatsby-remark-copy-linked-files/index.js
+++ b/node_modules/gatsby-remark-copy-linked-files/index.js
@@ -27,13 +27,10 @@ const validateDestinationDir = dir => {
     return true;
   } else if (typeof dir === `string`) {
     // need to pass dummy data for validation to work
-    return destinationIsValid(`${dir}/h/n`);
+    return destinationIsValid(`${dir}/n/h/a`);
   } else if (_.isFunction(dir)) {
     // need to pass dummy data for validation to work
-    return destinationIsValid(`${dir({
-      name: `n`,
-      hash: `h`
-    })}`);
+    return destinationIsValid(`${dir({ name: `n`, hash: `h`, absolutePath: `a` })}`);
   } else {
     return false;
   }
@@ -43,15 +40,11 @@ const defaultDestination = linkNode => `${linkNode.internal.contentDigest}/${lin
 
 const getDestination = (linkNode, dir) => {
   if (_.isFunction(dir)) {
-    // need to pass dummy data for validation to work
-    const isValidFunction = `${dir({
-      name: `n`,
-      hash: `h`
-    })}` !== `${dir({})}`;
-    return isValidFunction ? `${dir({
+    return `${dir({
       name: linkNode.name,
-      hash: linkNode.internal.contentDigest
-    })}.${linkNode.extension}` : `${dir()}/${defaultDestination(linkNode)}`;
+      hash: linkNode.internal.contentDigest,
+      absolutePath: linkNode.absolutePath,
+    })}.${linkNode.extension}`;
   } else if (_.isString(dir)) {
     return `${dir}/${defaultDestination(linkNode)}`;
   } else {
@@ -73,7 +66,10 @@ const newLinkURL = (linkNode, options, pathPrefix) => {
     destinationDir
   } = options;
   const destination = getDestination(linkNode, destinationDir);
-  return `${pathPrefix ? pathPrefix : ``}/${destination}`;
+  const startsWithSlash = destination.startsWith(`/`)
+  return `${pathPrefix ? pathPrefix : ``}${
+    startsWithSlash ? `` : `/`
+  }${destination}`
 };
 
 function toArray(buf) {

[karlhorky]

So this seems to be working / at parity with akabekobeko/npm-gatsby-remark-copy-relative-linked-files using this config with path.dirname and __dirname (not sure if this is what you meant):

-          {
-            resolve: 'gatsby-remark-copy-relative-linked-files',
-            options: {
-              filename: ({
-                name,
-                hash,
-                extension,
-              }: {
-                name: string;
-                hash: string;
-                extension: string;
-              }) => `${name}-${hash}.${extension}`,
-            },
-          },
+          {
+            resolve: 'gatsby-remark-copy-linked-files',
+            options: {
+              destinationDir: ({
+                name,
+                hash,
+                absolutePath,
+              }: {
+                name: string;
+                hash: string;
+                absolutePath: string;
+              }) => {
+                return `${path
+                  .dirname(absolutePath)
+                  .replace(`${__dirname}/src/pages/`, '')}/${name}-${hash}`;
+              },
+            },
+          },

[karlhorky]

So from my perspective, this could be merged! 🎉

Maybe there could be an example added to the examples section in the readme too, with the absolutePath option - not sure about other common use cases for this though... should this example also use path.dirname, .replace() and __dirname as in my example?

[LekoArts]

Yes, that's what I meant :) I'll want to add an example to the README for sure, can you try this instead of what you have?

path.dirname(path.relative(__dirname, absolutePath))

And

path.dirname(path.relative(path.join(__dirname, `src`, `pages`), absolutePath))

This should give back (for first example) the src/pages/.... path, right? And the second should be what you want to have? First is probably what most people would want, the directory path from the root, you want the paths relative to a nested dir. If that both works (the replace won't work on e.g. windows systems with backslashes) then use that instead

[karlhorky]

First is probably what most people would want, the directory path from the root

Oh, is this true? Given the following example, my assumption was that the path returned from the destinationDir function should NOT contain src/pages in it - because then that would also appear inside public/, which would be kind of strange I guess...

# save `my-awesome-pdf.pdf` to `public/downloads/2a0039f3a61f4510f41678438e4c863a/my-awesome-pdf.pdf`
destinationDir: f => `downloads/${f.hash}/${f.name}`

[LekoArts]

Nevermind, I read this again:

Note: The my-awesome-pdf.pdf file should be in the same directory as the markdown file.

I assumed you'd have the files in another directory. Then e.g. the src/pages use case it probably way more valid :)

Then I'll push a change to the README and I think we can

[LekoArts]

@karlhorky README should be good now?

[karlhorky]

could also rework all the messages in the tests to remove "root" and refer to "public" instead...

but we could also just leave it! 🤷‍♂️

[karlhorky]

Looking pretty good! Just some small suggestions.

[karlhorky]

Looking good, ready to merge?

[LekoArts]

You can use it with next version.

[karlhorky]

Nice, thanks for helping out so much here @LekoArts !

I'll upgrade once it's in a non-beta release.

[karlhorky]

Oh @LekoArts I just realized that there is still the issue of generated asset paths defined by gatsby-plugin-sharp and gatsby-remark-images, which are not yet customizable.

Now that there is a destinationDir with absolutePath in gatsby-remark-copy-linked-files, would it be possible to also get a corresponding option in gatsby-plugin-sharp and gatsby-remark-images?

I've opened an issue for this in 2020, but it was converted to a discussion (and then forgotten, like happens normally with discussions):

• gatsby-plugin-sharp / gatsby-remark-images: Set destination directory with destinationDir option #27153
• `gatsby-plugin-sharp` / `gatsby-remark-images`: Set destination directory with destinationDir option #32739

I'd be happy to take a shot at the implementation for this as well.

[karlhorky]

@LekoArts Ok, finally got around to testing gatsby-remark-copy-linked-files@5.24.0 with the example that we worked on to add to the readme:

// save `src/pages/custom-folder/my-awesome-pdf.pdf` to `public/custom-folder/my-awesome-pdf.pdf`
// Note: Import `path` to use this example https://nodejs.org/api/path.html
destinationDir: f => `${path.dirname(path.relative(path.join(__dirname, `src`, `pages`), f.absolutePath))}/${f.name}`

...and got some weird output:

error "gatsby-plugin-mdx" threw an error while running the onCreateNode lifecycle:

[gatsby-remark-copy-linked-files You have supplied an invalid destination directory. The destination directory must be a child but was: e=>`${s.default.dirname(s.default.relative(s.default.join(m,"src","pages"),e.absolutePath))}/${e.name}-${e.hash}`

When I logged the name, hash and absolutePath, I got the single-letter strings passed here:

gatsby/packages/gatsby-remark-copy-linked-files/src/index.js

Line 27 in f9dc16f

`${dir({ name: `n`, hash: `h`, absolutePath: `a` })}`

{
  resolve: 'gatsby-remark-copy-linked-files',
  options: {
    destinationDir: (f: {
      name: string;
      hash: string;
      absolutePath: string;
    }) =>
      
      console.log(f.name, f.hash, f.absolutePath) ||
      console.log(
        `${path.dirname(
          path.relative(
            path.join(__dirname, 'src', 'pages'),
            f.absolutePath,
          ),
        )}/${f.name}-${f.hash}`,
      )

yielded:

n h a
../../n-h

[karlhorky]

I'm working around it now in a different way, but I guess this is not desirable for users to have to figure out:

{
  resolve: 'gatsby-remark-copy-linked-files',
  options: {
    destinationDir: (f: {
      name: string;
      hash: string;
      absolutePath: string;
    }) =>
      // Avoid errors when Gatsby calls function using this data:
      // { name: n, hash: h, absolutePath: a }
      !f.absolutePath.includes('/')
        ? `${f.hash}/${f.name}`
        : `${path.dirname(
            path.relative(
              path.join(__dirname, 'src', 'pages'),
              f.absolutePath,
            ),
          )}/${f.name}-${f.hash}`,
  },
},