codidact / qpixel

Q&A-based community knowledge-sharing software
https://codidact.com
GNU Affero General Public License v3.0
385 stars 69 forks source link

Allow caching for sites without custom css and js #954

Closed trichoplax closed 1 year ago

trichoplax commented 1 year ago

Is your feature request related to a problem? Please describe. Sites with custom CSS and JS (for example codegolf.css and codegolf.js) have this cached by the browser. However, sites without custom CSS and JS get a 404 response (by design), which is not cached. This results in the requests being repeated on every page load.

Describe the solution you'd like Give every site a custom CSS and JS file in public/assets/community

Sites which do not require any customised styling or behaviour can leave these files empty, but having empty files rather than no files allows caching.

Describe alternatives you've considered We could instead change the HTML so that custom CSS and JS are not requested for communities that do not require them.

This would be better for communities that never change between having and not having custom CSS and JS, as there would never be any requests, not even the first request before caching.

This would be worse for communities that sometimes change between having and not having custom CSS and JS, as there would need to be a change to the HTML every time rather than just changing the CSS and JS.

Empty CSS and JS files keep changes simpler.

Additional context The difference is unlikely to be noticeable for most users. This will just reduce the number of requests being made to the server, and reduce the delays caused by slow loading on very slow connections (currently the page does not start to render until the 404 response returns).

cellio commented 1 year ago

Is it possible to have default files (which are empty), and if there's a community-specific one it overrides that default? This seems more future-proof than having copies of the empty files for each community, but I don't know how complicated it is and we could instead have the "create new community" code add them.

I defer to people with more knowledge, but this feels like something where there should be one set of (empty) files in source control, and the process of deploying a server should create community-specific copies where needed. But I don't know what's practical here, and I don't want to over-engineer for an infrequent situation.

ArtOfCode- commented 1 year ago

I've turned on caching for community assets in Cloudflare, which should mean that the 404 response is cached both at the CF edge servers and in client browsers. I've checked that it's being cached appropriately on mine.

trichoplax commented 1 year ago

@ArtOfCode- I'm still seeing the 404 response for meta.css and meta.js coming from the server rather than browser cache. Any reason it would take longer to take effect for me? I'm in the UK so I'm guessing not a different server. Using Firefox, getting no caching of the 404s either signed in or in private browsing without signing in.

ArtOfCode- commented 1 year ago

@trichoplax It could well. How exactly CF's edge caching works is best known only to the small ferrets that run around operating it... I'd give it 48-72 hours and see if it's working.

manassehkatz commented 1 year ago

I look at 404s as "problems to be fixed". While this is just my opinion, it leads me to two conclusions: 1 - either the code that generates a request that results in a 404 should be fixed or the requests should be satisfied, which can be done using empty files in this instance; 2 - while it may be that 404s can be cached, from my perspective a 404 should never be cached - i.e., you (the browser) keep trying in the hopes that the server finally resolved the problem. So I really think "empty" is better than 404 and it avoids any need for "special" caching as the browser will cache an empty file just fine.

trichoplax commented 1 year ago

Summary: I now see the HTML approach that @manassehkatz suggested on Discord as measurably best for the end user.


I see visible 404s as problems to be fixed, but if the page presented to the user works correctly then I see "hidden 404s" as just part of the language that the server speaks to the client.

I raised this issue because the 404s were not completely hidden. The user could not see them directly but they could see delayed page rendering.

I see these 3 approaches as all being valid:

Any of these will result in less waiting for an end user on a slow connection.

Rather than focus on the "correct" way to do things, where everyone will have a different opinion, I'd like to look at the practical outcomes for the end user.

After caching, all 3 approaches will be indistinguishable.

On first page load, or after cache expiry, both the 404 approach and the empty file approach mean the end user has to wait. The only way that never makes the end user wait is the HTML approach.

So the HTML approach is best for the end user, but makes a very occasional small amount of extra work for developers.

Of the other two options, both involve occasional slow page loads, and both introduce the potential for being out of date (new CSS or JS doesn't show up until the expiry of the cache - measures to force abandoning the cache early will involve changes to the HTML, at which point we may as well take the HTML approach anyway).

Based on this my preference is to make the requests for [community].css and [community].js conditional, so they only exist in the HTML for sites that have those custom files (currently Code Golf and Judaism).

trichoplax commented 1 year ago

There is a pull request (#962) proposing to change the 404s to 204s ("No content") to allow Selenium automated browser testing (which does regard hidden 404s as failures). Specifically commit 38f981e1.

That will address any remaining discomfort with using 404s, and I'll raise a separate issue for altering the HTML, as this issue was just to add caching for the 404s, which is now done (so this one can stay closed).

trichoplax commented 1 year ago

I've created #964 for the change to the HTML, so further discussion of whether to take that approach can happen there.