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

create one file cache per instance #84

Merged
merged 2 commits into from
Oct 16, 2020

Conversation

Rich-Harris
Copy link
Contributor

If you have multiple instances of sirv with different options...

const static_handler = sirv('static', {
  etag: true,
  setHeaders: res => {
    res.setHeader('cache-control', `public, max-age=0, must-revalidate`);
  }
});

const assets_handler = sirv('build/assets', {
  maxAge: 31536000,
  immutable: true
});

http.createServer((req, res) => {
  assets_handler(req, res, () => {
    static_handler((req, res) => {
      ssr(req, res);
    });
  });
}).listen(3000);

...they currently share a FILES cache, meaning that files can be intercepted by the wrong handler. For example, a request for an asset like /global.css will be handled by assets_handler which is only supposed to deal with immutable (hashed) files. Because of that, options like setHeaders are disregarded.

(I know this usage of setHeaders looks redundant, but I've found that explicitly adding must-revalidate rather than relying on maxAge: 0 is necessary, otherwise the browser doesn't send if-none-match.)

This PR fixes it by making a FILES cache per handler. Haven't added a test yet because I wasn't sure how that should look, since all the existing tests assume a single handler.

@lukeed
Copy link
Owner

lukeed commented Oct 16, 2020

Thanks, good catch 👍

And I think it makes sense to auto-append must-revalidate if options.maxAge === 0

@lukeed lukeed merged commit c69bbfb into lukeed:master Oct 16, 2020
@Rich-Harris Rich-Harris deleted the no-shared-cache branch October 16, 2020 16:49
@paulocoghi
Copy link
Contributor

What I did to maintain the single file cache for all instances was to use the full file path as the cache index.

This way, even if multiple files have the same URL in different instances, they certainly won't have the same absolute address in the file system.

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.

3 participants