jacobwb / hashover-next

This branch will be HashOver 2.0
GNU Affero General Public License v3.0
420 stars 87 forks source link

Enable HTTP caching (adds Cache-Control and Vary headers, changes some APIs from POST to GET, replaces time= with tz=) #320

Open da2x opened 2 years ago

da2x commented 2 years ago

This only enabled caching on some assets, and not any responses that change per-user or based on cookies. It means admins may be confused by their sites not immediately changing when they change a setting that modifies these assets. I decided to focus on the end user and their experience instead. You should’t need to change settings all that often, so this is a much smaller use case anyway. A compromise could be to disable caching for the admins, but this wouldn’t be a true representation of how users experience their sites. (Admins using HashOver are therefor expected to understand caching and how to turn it off in developer tools.)

I thought long and hard about where to put the caching headers. I decided they belonged where they’re used. I figured confused developers would look at the code to figure out why a changed setting didn’t apply as expected. Greeting them with the cache header should explain it, so it’s better for code readability. I can make it a function inside Setup instead if you hate this.

Bonus sneaking in along with the others:

Each change is its own commit, so you can merge just the parts you want.

da2x commented 2 years ago

I’m done with this now. The main comment loading is still not cachable. I’ll handle that in a separate package after this is merged.