FoalTS / foal

Full-featured Node.js framework 🚀
https://foalts.org/
MIT License
1.9k stars 142 forks source link

Possibility to set option to static serving (via express.static) #1221

Closed changke closed 1 year ago

changke commented 1 year ago

Hi,

my use case could actually be not so significant, but anyway :-)

I do use FoalTS to write traditional SSR apps, where also static files like CSS and JS are served by Foal, default to put them in the public folder.

Recently while debugging a CDN caching issue, I noticed those static resources all have a cache-control: public, max-age=0, max-age=<some other number> being set <- Yes, two different "max-age" values.

Multiple "max-age" values could be considered invalid, and it will depend on client to choose the actual number, my CDN in this case chose 0 -> so always missing cache.

On my VPS, I use Apache as reverse proxy server and the H5BP apache config to optimize caching, it will e.g. add some 1 year cache time to CSS files, it will merge the existing "cache-control" rather than replacing it, that was the cause of two "max-age" values.

But where does the first "max-age=0" come from? Turns out it was set by express.static() [1] which Foal uses to serve static files.

One can of course set some options to disable or overwrite this default value, but in Foal currently is not possible, since express.static() is called directly from create-app.ts [2]

My current workaround is

  1. Not use the default public folder for static files, create another one e.g. static
  2. Use the afterPreMiddlewares to manually call express.static(), as documented [3]

Code changes in index.ts

import * as express from 'express';

const app = await createApp(AppController, {
    afterPreMiddlewares: [express.static('static', {cacheControl: false})]
  });

Now all static files in the static folder are served by the new middleware without cache-control set, and I've got correct header with only one "max-age" value.

It is obviously verbose and not so elegant, so I wish there could be a way to set options at least to static serving middleware somehow.

Thanks for reading :-)

[1] https://expressjs.com/en/4x/api.html#express.static [2] https://github.com/FoalTS/foal/blob/1d1e9d8c4682501c854ce87165be30e43bd28c89/packages/core/src/express/create-app.ts#L97 [3] https://foalts.org/docs/common/expressjs#pre-and-post-express-middlewares

LoicPoullain commented 1 year ago

We could add the possibility to pass an option to the createApp static middleware.

Are you sure the { cacheControl: false } works? When looking at the documentation and the implementation of the middleware, I don't see a mention of this property:

changke commented 1 year ago

It was mentioned here: https://expressjs.com/en/resources/middleware/serve-static.html#:~:text=Range%20request%20header.-,cacheControl,-Enable%20or%20disable

And in my case it did work.

Thanks!

LoicPoullain commented 1 year ago

Thank you for your reply. The feature has been added in v4.2. 👍