Skip to content

Conversation

@nickvergessen
Copy link
Member

  • We noticed that on Talk quite many assets are loaded before Talk
  • So we tested to add prepend perf: Load talk UI earlier spreed#16375
  • But turns out that only prepends the file within talks list
  • It still loaded the 25 reference widgets app in alphabetical order before the main UI of Talk 🙈
Before After
grafik grafik

Checklist

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen added this to the Nextcloud 33 milestone Nov 20, 2025
@nickvergessen nickvergessen self-assigned this Nov 20, 2025
@nickvergessen nickvergessen requested a review from a team as a code owner November 20, 2025 15:14
@nickvergessen nickvergessen requested review from Altahrim and salmart-dev and removed request for a team November 20, 2025 15:14
@nickvergessen nickvergessen added the 3. to review Waiting for reviews label Nov 20, 2025
@susnux
Copy link
Contributor

susnux commented Nov 20, 2025

Isnt this on purpose?
So that you can prepand a script in your scripts list.
But this now does not prepand the script in the app scripts but it prepands it before all other scripts of other apps.

For me this sounds like a different use case where we should maybe have a order or priority option.

So before: use prepand to unshift a script in your app scripts (make a higher priority script for your app).
After: prepand increases the priority of all scripts of that application having side effects on all other apps.

// Make sure the scripts of prepended applications are prepended within the apps as well
$appDeps = self::$scriptDeps[$application] ?? [];
unset(self::$scriptDeps[$application]);
self::$scriptDeps = [$application => $appDeps] + self::$scriptDeps;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the behavior from unshift a script in the list of your scripts (e.g. because you need specific order) to give your app higher priority over all other apps.

I do not think that is bad, but in that case please add a documentation string to the prepand attribute that it will prepand all scripts of that app in the global scripts list.

@nickvergessen
Copy link
Member Author

Isnt this on purpose?
After: prepand increases the priority of all scripts of that application having side effects on all other apps.

Currently it's ordered "fifo". Since Talk/Spreed is rather late in the alphabet all other apps got triggered already even if they are neither the main page nor important for the content are loaded already via the RenderReferenceEvent event.

If talk similarly to calendar adds a single addScript in its boot() method, ALL files of Talk are in the front block:

Before After
grafik grafik

So yes, the "after" you outline is exactly what we want to achieve. On call/$token URL, after core for the main headers, the talk-main.js should be the next thing loaded, not 25 reference widget apps first. We can also add a different prepend parameter, so we can load talk-main.js prepended app wise and the rest of talk at the old position, but we need a way to get talk-main.js to be earlier.

@susnux
Copy link
Contributor

susnux commented Nov 20, 2025

Then as said: I think this is a good change.
But to prevent someone else has the same questions later could you add a small doc string on the docblock for that parameter?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants