Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#382 Cache busting, removing obsolete code for fonts #383

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

anneke
Copy link
Collaborator

@anneke anneke commented Oct 20, 2024

@bnijenhuis Used the script from https://bnijenhuis.nl/notes/cache-busting-in-eleventy/ but unfortunately I keep getting:

<link rel="stylesheet" href="/assets/css/style.css?">

Signed-off-by: Anneke Sinnema <[email protected]>
Signed-off-by: Anneke Sinnema <[email protected]>
@bnijenhuis
Copy link
Member

@anneke There are 2 issues here. First one is the folder structure that needs to be handled, second one is that this filter checks the modified date of the file, but because it's only a file with imports, it won't get modified when other files get modified. So it has to loop through all the files en get the latest modified date. I'll see if I can work on solving these issues.

@anneke
Copy link
Collaborator Author

anneke commented Oct 20, 2024

@bnijenhuis maybe the cache busting is most relevant for local development anyway, and hopefully we'll be done with most of that soon? :)

@bnijenhuis
Copy link
Member

@anneke It's useful for live as well, because it forces a CSS reload if anything changed in the CSS. So I think it'll still be useful...

@SleeplessByte
Copy link
Member

It should exist for all files that we change often and it's kinda meh that eleventy doesn't support this out of the box. In our case, the cache-key for the CSS file should be based on the compound of all css files. No need to traverse the imports @bnijenhuis, because we only have a single entry point that will always explode to all files (we don't want unused files anyway).

So either: MAX(changed_at) of glob **/*.(s)css, or (maybe later) a general purpose digester:

  // A cache to store the hashed file names
  const hashCache = {};

  // A cache buster if a file changes
  const prefixLength ="./src/site".length
  eleventyConfig.on('eleventy.beforeWatch', async (changedFiles) => {
    for(const file of changedFiles) {
      const relativePath = file.slice(prefixLength)
      delete hashCache[file]
    }
  });

  // A filter to dynamically hash asset file contents
  eleventyConfig.addFilter("digest", async (filePath)  => {
    // If we've already hashed this file, return the hash
    if(hashCache[filePath]) {
      return hashCache[filePath];
    }

    // Get the absolute path to the file inside of src/site
    const absolutePath = path.join(__dirname, 'src/site', filePath);

    // Digest the file
    const fileBuffer = fs.readFileSync(absolutePath);
    const hash = crypto.createHash('sha256').update(fileBuffer).digest('hex');
    const relativePath = filePath.slice(0, path.basename(filePath).length * -1)
    const digestFileName = `${relativePath}${hash}-${path.basename(filePath)}`;

    // See if the digest file exists in the output folder _site
    const digestFilePath = path.join(__dirname, '_site', digestFileName);
    hashCache[filePath] = digestFileName;

    if(!fs.existsSync(digestFilePath)) {
      if(!fs.existsSync(path.dirname(digestFilePath))) {
        fs.mkdirSync(path.dirname(digestFilePath), { recursive: true });
      }
      fs.copyFileSync(absolutePath, digestFilePath);
    }

    // Return the digest file name
    return digestFileName;
  })

For the css file we'd need to digest all the css files together. Can be done by appending all the content (fileBuffer), or digesting each css file individually and then getting the sha256 of all the digests.

The upside of such a filter is that we can start including assets such as images that change often using { file path | digest } and it will JustWork™, as well as set Cache-Control headers for netlify to immutable + 1 year for those assets.

For today I recommend MAX(changed_at) of glob **/*.(s)css.

@bnijenhuis
Copy link
Member

@SleeplessByte (both) sound great.

@SleeplessByte
Copy link
Member

@bnijenhuis the code for the easy one would be something like:

const path = require('node:path');
const { glob, stat } = require('node:fs/promises');

const entries = await glob('**/*.css')
const times = await Promise.all(entries.map((entry) => stat(entry).then((stats) => stats.mtime)))
const lastModifiedAt = Math.max(...times);

// Do something with that
eleventyConfig.on('eleventy.beforeWatch', async (changedFiles) => {
  for(const file of changedFiles) {
    if (path.extname(file) === '.css') {
      // Do something with current time
    }
  }
});

Are you able and willing to turn this into something working?

@bnijenhuis
Copy link
Member

@SleeplessByte I'll have a look at it, but I recommend losing the JS cache busting code...

@anneke
Copy link
Collaborator Author

anneke commented Oct 20, 2024

Yes removing that is fine anyway Bernard!

@bnijenhuis
Copy link
Member

@SleeplessByte Didn't quite get it to work yet. The awaits can only be used in the events, or maybe I'm missing something. Haven't worked with Eleventy in a while.

@SleeplessByte
Copy link
Member

Instead of the promises variants, there should be blocking ones too

@edwinm edwinm requested a review from a team as a code owner October 22, 2024 07:29
@edwinm edwinm requested review from SleeplessByte and removed request for a team October 22, 2024 07:29
@edwinm edwinm marked this pull request as draft October 22, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants