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

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

Open
2 of 6 tasks
kamil-neutrica opened this issue Nov 24, 2023 · 4 comments
Open
2 of 6 tasks

Comments

@kamil-neutrica
Copy link

Issue type (mark with x)

  • πŸ€” Question
  • πŸ› Bug report
  • 🎁 Feature request
  • πŸ€·β€β™€οΈ Other

Version (mark with x)

  • 2️⃣ v2.24.2
  • 3️⃣ v3.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("[email protected]/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):

  • Create React Portlet
  • Create React Component
  • Import .scss file into React Component Javascript file
  • Run npm run build
  • See generated css module contenst: build/node/packageRunBuild/resources/css/{css file name}.css.js
  • CSS Module has href without cache busting fragment

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
Copy link
Member

@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
Copy link
Member

izaera commented Nov 29, 2023

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 πŸ˜… ) 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... πŸ€” 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
Copy link
Member

izaera commented Nov 29, 2023

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
Copy link
Author

kamil-neutrica commented Dec 4, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants