WIP defer and async loading of JS in themes and templates#11202
WIP defer and async loading of JS in themes and templates#11202sonOfRa wants to merge 1 commit intokeycloak:mainfrom
Conversation
| <script src="${url.resourcesPath}/${script}" type="text/javascript"></script> | ||
| </#list> | ||
| </#if> | ||
| <#if properties.asyncScripts?has_content> |
There was a problem hiding this comment.
This allows a theme extending base (or another theme that extends base) to add scripts that should be loaded with the async or defer parameters
|
Obviously this is incomplete (there are several more theme bases that would need to incorporate the changes), but for a basic discussion whether or not this is a feature that might be accepted, this should be enough. The basic idea is introducing a new |
jonkoops
left a comment
There was a problem hiding this comment.
I have no problem with adding this feature, but someone has to take a look at the Java code.
|
I'm wondering how much sense this makes as more and more themes are moving away from using the velocity templates |
That is not something I was previously aware of! For our use case, this is mostly about custom authenticators, but also to some degree about themes for the login/register/otp forms. Those are, in our use case mostly built around looking at the base or keycloak templates, and modifying them to reflect our needs. Is that functionality going to go away or be replaced with another mechanism? If that is the case, then I would tend to agree, and we would start moving to whatever is the intended replacement for this mechanism, rather than working with an older approach |
|
I think what we are going to end up with is that all scripts included in the new UIs (see keycloak/keycloak-ui) will end up with So perhaps it might be a better solution to not merge this PR. |
|
Since this PR only affects login, we might still want to consider it. Login flow will probably be the last thing to move to keycloak-ui. And even then, I'm not sure exactly what we are going to do. There is a good argument for keeping login lean, quick, and universally compatible. The current implementation does that nicely. |
stianst
left a comment
There was a problem hiding this comment.
I'm going to close this as all it does is allow a custom theme to load scripts asynchronously, that can easily be done by modifying the template.ftl without adding this additional complexity in KC.
|
@stianst we'll eventually have to create a more flexible system to load assets, which could include this feature. I've drafted an idea on how that could work. |
This is a start for an implementation of the change I suggested in #11201