divriots / jampack

Optimizes static websites for best user experience and best Core Web Vitals scores.
https://jampack.divriots.com/
MIT License
1.62k stars 24 forks source link

External image support #38

Closed mneumegen closed 11 months ago

mneumegen commented 1 year ago

Congrats on launching jampack! It's an absolute game-changer in automating a lot of tedious performance optimizations. I'm a big fan.

The one issue preventing me from using this more is support for external images.

Currently, jampack will add decoding="async" loading="lazy" to an externally referenced image which is an excellent start. I'd love a CLI option to process external images further.

At a minimum, I'm looking to add missing width/height attributes without touching the src. Even better (or as an option) is downloading the external image, processing it, and changing the src to serve it locally.

Would you happen to have any thoughts about this?

georges-gomes commented 1 year ago

Hi!

Thank for the kind words :)

add missing width/height attributes without touching the src.

I think I can do that and it exist libs to do that without downloading the entire images... I'm adding that to my bucket list 👍

downloading the external image, processing it, and changing the src to serve it locally.

Out of curiosity, why would you do that? Is that because images are served from a CMS and can't be responsive?

mneumegen commented 1 year ago

Great stuff!

Downloading the image and serving it locally isn't a huge blocker honestly, it would be nice to have the option though. The use case is we try to keep assets out of our repositories so they're lean. It leads us to storing assets on something like Cloudinary or S3. When the site builds, I'm ok with them pulling in at that point, especially if I know I can improve the performance with jampack, just as long as they're not in the repo.

Hope that makes sense.

silveltman commented 1 year ago

Great work on Jampack indeed!

For me too adding width and heights to external images would be very useful too. This would prevent cumulative layout shift (CLS).

And I would also love the feature of > downloading external image > save locally > create sizes with srcset. This would allow me to have responsive external images.

georges-gomes commented 1 year ago

Understood loud and clear and I'm excited to implement it soon 💪

silveltman commented 1 year ago

In the meantime I created a little script to save external images locally.

Why I did this: I want the script to also update the original image url (which lives in a .md file in my Astro content collection, in my case), so that it is permanent and the local file is used for each subsequent build.

I assumed Jampack would probably never do this, since it runs AFTER the build and only on the dist folder.

Anyway, here is my script:

// --------------------------
// A little script to save all external images locally and update the markdown files
// --------------------------

import { promises as fsPromises } from 'fs'
import fs from 'fs'
import glob from 'glob'
import crypto from 'crypto'

const MARKDOWN_FILES = glob.sync('./src/content/**/*.md')
const FILE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'webp', 'avif', 'svg']
const PUBLIC_OUTPUT_DIR = 'remote-image-cache'

// Get markdown content as string
function getMarkdownContent(filePath) {
  return fs.readFileSync(filePath, 'utf-8')
}

// Check if url has a file extension sepcified in FILE_EXTENSIONS
function hasFileExtension(url) {
  return FILE_EXTENSIONS.some((extension) =>
    url.toLowerCase().includes(`.${extension.toLowerCase()}`)
  )
}

// Get file extension from url
function getFileExtension(url) {
  return FILE_EXTENSIONS.find((extension) =>
    url.toLowerCase().includes(`.${extension.toLowerCase()}`)
  )
}

// Extract all external urls from markdown content
function extractImageUrls(contentString) {
  const regex = /(https?:\/\/[^\s]+)/g
  const matches = contentString.matchAll(regex)
  const urls = [...matches].map((match) => match[1])
  const imageUrls = urls.filter((url) => hasFileExtension(url))
  return imageUrls
}

// Download image from url and save to local filesystem
async function downloadImage(url) {
  const extension = getFileExtension(url)
  const randomString = crypto.randomBytes(16).toString('hex')
  const filename = `${randomString}.${extension}`
  const outputPath = `./public/${PUBLIC_OUTPUT_DIR}/${filename}`

  const response = await fetch(url)
  const blob = await response.blob()
  const arrayBuffer = await blob.arrayBuffer()
  const buffer = Buffer.from(arrayBuffer)

  await fsPromises.writeFile(outputPath, buffer)
  return `/${PUBLIC_OUTPUT_DIR}/${filename}`
}

// Get markdown content, extract image urls, download images and update markdown content
async function runAll(filePath) {
  const content = getMarkdownContent(filePath)
  const urls = extractImageUrls(content)
  let newContent = content
  for (const url of urls) {
    const newUrl = await downloadImage(url)
    newContent = newContent.replace(url, newUrl)
  }
  fs.writeFileSync(filePath, newContent)
}

// Run script for all markdown files
MARKDOWN_FILES.forEach((filePath) => {
  runAll(filePath)
})

Any tips on how to improve would be more than welcome, I might publish it as an Astro integration later.

georges-gomes commented 1 year ago

@silveltman @mneumegen A first version of the external image support has been released : v0.12.0

It's off by default but you can active it with a config file. You can read more about it here: https://jampack.divriots.com/devlog/external-images/

I made the extra mile in implementing an HTTP cache so images are not downloaded all the time.

For the moment, the cache is reset with every new version of jampack but I will improve that later.

There is problably some bugs. I would love to have your feedback if you can run it on your websites 🙏

Enjoy!

georges-gomes commented 1 year ago

@mneumegen I looked at your website (very nice by the way) and you seem to use imgix as image CDN.

I thought I could implement a process option that would use the resizing capabilities of imgix instead downloading and resizing locally.

jampack would fill image dimensions and generate srcset with imgix urls.

It would tremendously reduce the processing time of jampack and you would get 99% of the benefits.

Let me know your thoughts.

bendrick92 commented 1 year ago

@georges-gomes awesome work getting support for external image optimization added! I'm working on POC'ing a rewrite of one of my sites in Astro and I was having issues getting the astro-imagetools to parse external images in my Markdown.

I installed jampack and ran my build locally and it looks like it found, downloaded, and optimized the remote image! Unfortunately, because my Markdown file is nested under a posts directory, it looks like the relative path resolution isn't working quite right:

My jampack.config.mjs:

export default {
    image: {
        compress: true,
        external: {
            process: 'download'
        }
    },
};

The original, remote Markdown image located at src > pages > posts > this-is-a-test.md:

![A remote image](https://assets.bpwalters.com/images/bens_car_blog/wrx_replacement/model3.jpg)

The optimized result from jampack from dist > posts > this-is-a-test > index.html:

<p><img alt="A remote image" decoding="async" fetchpriority="high" src="/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg"></p>

The jampack output throws erro Can't find img on disk src="/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg" since the _jampack image folder gets placed one level up at the root of dist.

georges-gomes commented 1 year ago

Hi @bendrick92 Thanks for reporting. I also had this issue when the site is not served as root. I will release a version with local _jampack folders until I find a clever way to do this in a single folder.

georges-gomes commented 1 year ago

@bendrick92 please try version 0.12.2 - it creates multiple _jampack folders next to the .html pages. It's not ideal but at least it should be working.

georges-gomes commented 1 year ago

@bendrick92

The jampack output throws erro Can't find img on disk src="/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg" since the _jampack image folder gets placed one level up at the root of dist.

This is maybe another bug actually.

In which folder to you run jampack? If you run jampack ./dist the dist/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg should exist and be found. There must be a bug somewhere.

If you can share more details on your setup, that would help.

Thanks 🙏

bendrick92 commented 1 year ago

@bendrick92

The jampack output throws erro Can't find img on disk src="/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg" since the _jampack image folder gets placed one level up at the root of dist.

This is maybe another bug actually.

In which folder to you run jampack? If you run jampack ./dist the dist/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg should exist and be found. There must be a bug somewhere.

If you can share more details on your setup, that would help.

Thanks 🙏

For sure! I'm running a fork off of the astro-netlify-cms-starter repo.

I updated my package.json build command from "build": "astro build", to "build": "astro build && jampack ./dist",.

I can confirm the dist/_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg is generated, but because my posts collection gets compiled to a subdirectory, it looks like jampack is assuming the relative reference is one directory deep:

SCR-20230529-dld

I was able to get the image to render if I changed the relative path to "../../_jampack/e0a3fe918067c2a86fa60222a7bf400b.jpg"

Hopefully I'm not missing something in the configuration?

silveltman commented 1 year ago

@mneumegen I looked at your website (very nice by the way) and you seem to use imgix as image CDN.

I thought I could implement a process option that would use the resizing capabilities of imgix instead downloading and resizing locally.

jampack would fill image dimensions and generate srcset with imgix urls.

It would tremendously reduce the processing time of jampack and you would get 99% of the benefits.

Let me know your thoughts.

I would really like this feature for Cloudinary, since it is first-class supported by the CMS I use (which, believe it or not, is actually @mneumegen 's Cloudcannon haha (amazing product and company!))

Also, I'll give the new feature a try this week and report any problems I might encounter

georges-gomes commented 1 year ago

@bendrick92 Thanks for the details!

If you try version 0.12.2, it's fixed 👍

bendrick92 commented 1 year ago

@bendrick92 Thanks for the details!

If you try version 0.12.2, it's fixed 👍

Fantastic! Thank you for the quick fix, you rock! I’ll try it out and report back if I have any issues.

georges-gomes commented 1 year ago

@mneumegen I looked at your website (very nice by the way) and you seem to use imgix as image CDN. I thought I could implement a process option that would use the resizing capabilities of imgix instead downloading and resizing locally. jampack would fill image dimensions and generate srcset with imgix urls. It would tremendously reduce the processing time of jampack and you would get 99% of the benefits. Let me know your thoughts.

I would really like this feature for Cloudinary, since it is first-class supported by the CMS I use (which, believe it or not, is actually @mneumegen 's Cloudcannon haha (amazing product and company!))

Also, I'll give the new feature a try this week and report any problems I might encounter

Yes, I will try to make it work with for a number of CDN images. First 2 will be Cloudinary and Imgix 👍

georges-gomes commented 1 year ago

@muryoh has added support for image CDNs. Currently very little documentation. You can enable it:

image: {
    cdn: {
      process:
        | 'off' //default
        | 'optimize';
      src_include: RegExp;
      src_exclude: RegExp | null;
    };

This will detect urls that uses an image CDN. Currently supporting:

Adobe Dynamic Media (Scene7) Builder.io Bunny.net Cloudflare Contentful Cloudinary Directus Imgix, including Unsplash, DatoCMS, Sanity and Prismic Kontent.ai Shopify Storyblok Vercel / Next.js WordPress.com and Jetpack Site Accelerator

(thanks to unpic js library)

A few limitations for the moment: 1) width and height values need to be in <img> tag 2) you can't have a custom domain to your images because it uses the url to detect the CDN provider

I don't know if these limitations are showstopper for you @mneumegen @silveltman Let me know, we can improve it.

Cheers

georges-gomes commented 11 months ago

This seems to be supported in the most part. We will open more issues if needed