liferay / liferay-frontend-projects

A monorepo containing assorted Frontend Infrastructure Team projects
Other
66 stars 67 forks source link

Cache Busting issues with modules loaded via liferay-npm-bundler-loader-css-loader #1182

Open kamil-neutrica opened 7 months ago

kamil-neutrica commented 7 months ago

Issue type (mark with x)

Version (mark with x)

Description

I have .scss file imported in react component via: import './../../css/sector.scss';

For this file i have a module sector.scss.js generated with contents like given below:

!function () {
    var e = document.createElement("link");

    function t() {
        Liferay.Loader.define("com-test-web@1.18.1/css/sector.scss", ["module", "exports", "require"], (function (t, s, o) {
            window;
            t.exports = e
        }))
    }

    e.setAttribute("rel", "stylesheet"), e.setAttribute("type", "text/css"), e.setAttribute("href", Liferay.ThemeDisplay.getPathContext() + "/o/com-test-web/css/sector.css"), e.onload = t, e.onerror = function () {
        console.warn("Unable to load /o/com-test-web/css/sector.css. However, its .js module will still be defined to avoid breaking execution flow (expect some visual degradation)."), t()
    }, document.querySelector("head").appendChild(e)
}();

It adds link element with href without any timestamp (eg. /o/com-test-web/css/sector.css), version or cache busting suffix. It makes it impossible to properly cache those kind of files via cdn.

Desired behavior: Generated href should consist cache busting fragment: timestamp, module version or file hash. For example: /o/com-test-web/css/sector.css?t=1686646309656

Current behavior: Generated href consists no cache busting fragment. What is more, Liferay server sets header to: Cache-Control: max-age=315360000, public Combination of those two issues makes it impossible to properly cache those kind of files via cdn.

Repro instructions (if applicable):

Other information (environment, versions etc): LINUX, com.liferay.gradle.plugins.workspace version: 3.4.27 @liferay/npm-scripts version 47.0.0 liferay-npm-bundler-loader-css-loader version: 2.24.2 Liferay version: 7.4.3.33-ga33

bryceosterhaus commented 7 months ago

@izaera have we run into this before? Seems like we should add some sort of mechanism, but I'm surprised if we haven't run into something like this before

izaera commented 7 months ago

Well, this is clearly a server error. Sending Cache-Control: max-age=315360000, public for a resource than can change in the future is incorrect. And trying to fix it by appending a t parameter from the client is not the way to fix it as I have argued several times (apparently it's the issue of the month :sweat_smile: ) because it breaks the HTTP semantics.

Said that I understand there may be situations where the frontend needs to fix what the servers are doing wrong and we may want to provide tools for that. Meaning that we could add an option to the loader to add a parameter t to force cache cleaning and let the user decide whether he wants to break HTTP semantics or not.

However, as you said, @bryceosterhaus, this is the first time we see this behavior and nobody has ever complaint about it, so I'm wondering if this has something to do with the server caching configuration... :thinking: Specifically I'm thinking about settings in portal.properties that may lead to infinite caching.

CDNs can influence this too.

And if all that is discarded, it might be an issue with some DXP version, of course.

izaera commented 7 months ago

timestamp, module version or file hash.

This sounds much better to me than a t parameter, because it doesn't break HTTP semantics if done well. However, I'm not 100% sure if it could be done because it's possible that the CSS file is not generated by the bundler at all, so changing its name could have side effects (think of a CSS coming with a third party dependency, for example).

kamil-neutrica commented 7 months ago

We have solved this issue temporarily by lowering Cache-Control TTL on our ingress.

In general, long lasting Cache-Control was caused by default Liferay Header Filter configuration. See below:

<filter>
        <filter-name>Header Filter</filter-name>
        <filter-class>com.liferay.portal.servlet.filters.header.HeaderFilter</filter-class>
        <init-param>
            <param-name>url-regex-ignore-pattern</param-name>
            <param-value>.+/-/.+</param-value>
        </init-param>
        <init-param>
            <param-name>Cache-Control</param-name>
            <param-value>max-age=315360000, public</param-value>
        </init-param>
        <init-param>
            <param-name>Expires</param-name>
            <param-value>315360000</param-value>
        </init-param>
    </filter>

<filter-mapping>
  <filter-name>Header Filter</filter-name>
  <url-pattern>*.css</url-pattern>
</filter-mapping>

I predict such default filters configuration assumes .css URI has format like:

Without this cache busting part there is a risk, that in some environments developers cannot update .css for users by deploying new OSGI module version. CSS stays cached on the browser side for time given in Cache Control header. It can cause many hard to resolve (time and environment dependent) bugs.

Lets see Liferay.dev Portal as an example:

The main risk here is that changes made to the CSS file won't be reflected for users until their browsers refresh the cache after the specified max-age duration. Without cache busting, you have limited control over when users receive updated styles. If you make frequent changes to the CSS, users might experience outdated styles until the cache expires. If there are critical updates (imagine payment module bug) or fixes in your CSS, users may experience issues until their browser fetches the latest version.