solution-loisir / markdown-it-eleventy-img

A markdown-it plugin that processes images through the eleventy-img plugin. Can be used in any projects that use markdown-it.
Apache License 2.0
54 stars 5 forks source link

Feature request: Ability to manage remote URLs #2

Closed marcus-crane closed 2 years ago

marcus-crane commented 2 years ago

Hey there!

First off, thanks for making this plugin! It's nice to be able to keep website content as SSG agnostic as is reasonable 🙂

I'm not sure if it's expected but I was a bit surprised to see this plugin blow up when I tried using a remote URL, of which I have quite a few (I'm midway through migrating to Eleventy at the moment)

I can see that everything is fed into statsSync which has a check for remote URLs visible here: https://github.com/11ty/eleventy-img/blob/e51ad8e1da4a7e6528f3cc8f4b682972ba402a67/img.js#L545

The trouble with the recommended method (statsByDimensionsSync) however is that you need to provide an explicit height and width and you can't just pass in null to get automatic sizing.

Given that we have made a call to Image, we actually have this metadata so something like this is technically possible (although hacky):

const img = Image(src, options)

const fileTypes = Object.keys(img)
const firstType = fileTypes[0]
const largestImage = img[firstType].length - 1
// We just need "some" size so we'll fetch the largest available
const { height, width } = img[firstType][largestImage]

let metadata;

if (!Image.Util.isRemoteUrl(src)) {
  metadata = Image.statsSync(src, options)
} else {
  metadata = Image.statsByDimensionsSync(src, height, width, options)
}

This does actually work but the trouble I had is that Image technically returns a Promise.

I note that you mentioned that only synchronous calls are allowed within markdown-it so trying to await Image had no luck and neither did trying to chain Image.then(img => blah).

Well, functionally it worked but I would just end up with [Object promise] instead of my images when pages rendered 😅

Anyway, in the meantime, I'm happily using this plugin with the following renderImage method so I'm not in any particular rush for this issue to be resolved:

renderImage(image, attributes) {
  const [ Image, options ] = image;
  const [ src, attrs ] = attributes;

  Image(src, options)

  if (!Image.Util.isRemoteUrl(src)) {
    metadata = Image.statsSync(src, options)

    const imageMarkup = Image.generateHTML(metadata, attrs, {
      whitespaceMode: "inline"
    })

    return imageMarkup
  }
  return `<img src="${src}" alt="${attrs.alt}">`;
}

Nothing fancy, it just falls back to a regular img if it detects a remote URL using eleventy-img's built in Util.isRemoteURL method

Hopefully my digging around above gives a bit of an idea of how you might go about resolving this and if I can get something fully working, I'll have a go at submitting a PR 🙂

solution-loisir commented 2 years ago

Hi, thanks for pointing this out. To be honest, I had only tested with local images (my bad). It seems like there's no easy way out of this one. I was able to reproduce and just like you describe I got: "Error: statsSync is not supported with remote sources. Use statsByDimensionsSync instead." Now I went searching on the markdown-it side and found this comment hinting at a workaround hack to execute asynchronous processes.

I will need to think this over. So far, I can see two solution paths. Either somehow make this plugin async friendly so it can use eleventy-img async API or find a way to temporary import remote images just the time to process them. There is surely a third way I don't see yet. Thanks again for your input!

solution-loisir commented 2 years ago

So, after digging around I came to the obvious conclusion that processing remote images will always be asynchronous :sweat_smile:! That with the synchronous nature of markdown-it leaves us with statsByDimensionsSync or dark magic (and I don't want to go that way). statsByDimensionsSync can't be used as the default markdown-it eleventy-img rendering for the reasons you evoked. I think that the sanest for now is to revert to the native markdown-it renderer as a fallback whenever a remote image is encountered (which is very close to what you did as a workaround)!

You may upgrade to v0.7.0 and get this behavior for free. Also put a note about the limitations here. If you have other ideas you're always welcome! I will keep thinking about it, but in the meantime there shouldn't be anymore errors thrown over remote images in the default behaviour! :joy:

marcus-crane commented 2 years ago

Oh wow, thanks for that quick response! Falling back to the markdown-it render is perfectly fine with me since I don't link to remote images too often in blog posts. I guess I could also just pull in images locally as well to avoid link rot as well 🤔

Thanks for investigating!

antgel commented 1 year ago

Edit: Not only are my images remote, my markdown containing the image tags is remote as well, coming in from a JavaScript Data File then in the Nunjucks template I have something like:

<div>{{ post.attributes.content | markdown | safe }}</div>

Will that even work with the workaround at the bottom?

Original comment follows:

Hi, I know the issue is technically "closed", but this seems like the most appropriate place for my question. I have skimmed the above but it's late here so forgive me if I'm missing something.

My use case is connecting 11ty to a headless CMS e.g. Strapi. I have Strapi Media Library configured to make images accessible via S3 / CloudFront, so that 11ty can consume them. This works well with the standard eleventy-img plugin.

However, what I'd like to do is to be able to have, say, a markdown field in Strapi that contains a blog post with text and yes, a bunch of images. Is it correct reading the above, that this is basically impossible with the libraries that all this is built on?

My users are non-techie; they can work with Strapi as a CMS backend, but I can't expect them to start uploading files to our 11ty git repository, it kind of defeats the purpose of Strapi as a (non-geek) user-friendly interface.

Grateful for any pointers, ideas, workarounds.

I just thought of the following after reading @solution-loisir comment:

find a way to temporary import remote images

I use an 11ty JavaScript Data File to get the data from Strapi using Axios. It's ugly, but would it be feasible in that file to parse the returned Markdown field, get all the images with Axios and save them in a temporary directory, substitute the paths in the Markdown field to the temporary directory?

If this would work, I'd take the hit on clunkiness because working software is the name of the game.

solution-loisir commented 1 year ago

Hi @antgel, thanks for your interest! The limitations here are coming from the synchronous nature of the Markdown-it parser. So it is actually impossible to accomplish asynchronous tasks (like fetching remote images) as part of the parsing process. Mind you, it is also that very nature that makes Markdown-it so stable and fast.

Anyhow, for your use case you could, as you said, import remote sources so you can parse them locally, but I guess it would slow down your build quite a bit.

As an alternative, you could come up with a Nunjucks sortcode and use eleventy-img asynchronous api. That would solve the problem as you could fetch the images directly from the s3 bucket. I'm guessing that if your users are comfortable writing markdown, they would be OK learning an extra token for images, but I'll let you assess that. This plugin is mainly an ergonomic solution for using eleventy-img with the markdown image syntax, but it clearly shows its limitations here.

I don't think there's an issue importing markdown as text from a JavaScript data file, but it's something I've never tested. Let me know if its a problem.

Feel free to open a discussion if you want pursue conversation. I sincerely hope you find a solution that meets the needs of your users while remaining maintainable. :smile:

antgel commented 1 year ago

Thank you! Let's continue at https://github.com/solution-loisir/markdown-it-eleventy-img/discussions/59.