Bug 36674 - Lazy load api-client JS files
Summary: Lazy load api-client JS files
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Paul Derscheid
QA Contact: Lisette Scheer
URL:
Keywords: additional_work_needed, release-notes-needed
Depends on:
Blocks:
 
Reported: 2024-04-23 09:52 UTC by Jonathan Druart
Modified: 2025-11-13 11:27 UTC (History)
3 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 36674: Lazy-load intranet API clients via proxy (11.51 KB, patch)
2025-09-18 10:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 36674: Lazy-load intranet API clients via proxy (11.58 KB, patch)
2025-09-19 10:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36674: Remove duplication (2.88 KB, patch)
2025-09-19 10:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36674: Remove duplication (2.94 KB, patch)
2025-09-26 17:15 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 36674: Lazy-load intranet API clients via proxy (11.64 KB, patch)
2025-09-26 17:16 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 36674: Remove duplication (2.94 KB, patch)
2025-10-13 10:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36674: Fix warning (1.52 KB, patch)
2025-10-23 09:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36674: Revert "Bug 36674: Remove duplication" (2.89 KB, patch)
2025-10-23 11:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36674: (follow-up) Stop emitting Cypress API client source maps (1.52 KB, patch)
2025-11-04 13:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 36674: (follow-up) Stop emitting Cypress API client source maps (1.46 KB, patch)
2025-11-06 10:51 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 36674: (follow-up) Stop emitting Cypress API client source maps (1.54 KB, patch)
2025-11-13 11:27 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-04-23 09:52:08 UTC
js/fetch/api-client.js is fetching all the api-client.js files every time a page is hit in Koha.
We should lazy-load instead. Certainly using import()?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import
Comment 1 Paul Derscheid 2025-09-18 10:09:27 UTC
Created attachment 186551 [details] [review]
Bug 36674: Lazy-load intranet API clients via proxy

Replaced the eager imports in the intranet APIClient aggregator with
lazy proxies so individual client modules are only fetched the first
time they are actually used.

Documented the pattern in the createClientProxy JSDoc, and kept the legacy
synchronous API intact by forwarding property access, method calls, and
promise chaining. The Vue aggregator remains on static imports for now
because the rspack bundles already would handle chunking (which we need to configure)
and would require broader consumer changes.

This is a bit hacky, but only touches the api-client itself, which I like.
Otherwise we would have to adjust all consumers.

Test plan:
1. Log in to the staff interface and visit Admin → System preferences.
2. Open DevTools, Network tab, and enable “Disable cache”.
3. Trigger an API call (e.g. edit a preference and click “Save”, or run
await APIClient.sysprefs.sysprefs.update_all({}).catch(()=>{}); from
the console).
4. Confirm a new request for js/fetch/system-preferences-api-client.js
appears and the page still behaves as before.
5. Theoretically we would need to test all call sites..
6. Sign-off
Comment 2 Jonathan Druart 2025-09-19 10:33:18 UTC
Created attachment 186616 [details] [review]
Bug 36674: Lazy-load intranet API clients via proxy

Replaced the eager imports in the intranet APIClient aggregator with
lazy proxies so individual client modules are only fetched the first
time they are actually used.

Documented the pattern in the createClientProxy JSDoc, and kept the legacy
synchronous API intact by forwarding property access, method calls, and
promise chaining. The Vue aggregator remains on static imports for now
because the rspack bundles already would handle chunking (which we need to configure)
and would require broader consumer changes.

This is a bit hacky, but only touches the api-client itself, which I like.
Otherwise we would have to adjust all consumers.

Test plan:
1. Log in to the staff interface and visit Admin → System preferences.
2. Open DevTools, Network tab, and enable “Disable cache”.
3. Trigger an API call (e.g. edit a preference and click “Save”, or run
await APIClient.sysprefs.sysprefs.update_all({}).catch(()=>{}); from
the console).
4. Confirm a new request for js/fetch/system-preferences-api-client.js
appears and the page still behaves as before.
5. Theoretically we would need to test all call sites..
6. Sign-off

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Jonathan Druart 2025-09-19 10:33:20 UTC
Created attachment 186617 [details] [review]
Bug 36674: Remove duplication

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Jonathan Druart 2025-09-19 10:33:33 UTC
I have to admit I don't understand all what is doing the first patch but it's working very well!
Comment 5 Lisette Scheer 2025-09-26 17:15:50 UTC
Created attachment 186994 [details] [review]
Bug 36674: Remove duplication

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 6 Lisette Scheer 2025-09-26 17:16:25 UTC
Created attachment 186995 [details] [review]
Bug 36674: Lazy-load intranet API clients via proxy

Replaced the eager imports in the intranet APIClient aggregator with
lazy proxies so individual client modules are only fetched the first
time they are actually used.

Documented the pattern in the createClientProxy JSDoc, and kept the legacy
synchronous API intact by forwarding property access, method calls, and
promise chaining. The Vue aggregator remains on static imports for now
because the rspack bundles already would handle chunking (which we need to configure)
and would require broader consumer changes.

This is a bit hacky, but only touches the api-client itself, which I like.
Otherwise we would have to adjust all consumers.

Test plan:
1. Log in to the staff interface and visit Admin → System preferences.
2. Open DevTools, Network tab, and enable “Disable cache”.
3. Trigger an API call (e.g. edit a preference and click “Save”, or run
await APIClient.sysprefs.sysprefs.update_all({}).catch(()=>{}); from
the console).
4. Confirm a new request for js/fetch/system-preferences-api-client.js
appears and the page still behaves as before.
5. Theoretically we would need to test all call sites..
6. Sign-off

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 7 Lisette Scheer 2025-09-26 17:17:19 UTC
Thanks for all the notes in /koha-tmpl/intranet-tmpl/prog/js/fetch/api-client.js that'll be really helpful for future developers.
Comment 8 Lucas Gass (lukeg) 2025-10-10 18:20:03 UTC
We need a rebase here now.
Comment 9 Jonathan Druart 2025-10-13 10:16:07 UTC
Created attachment 187826 [details] [review]
Bug 36674: Remove duplication

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Comment 10 Jonathan Druart 2025-10-13 10:16:50 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2025-10-13 10:17:11 UTC
(In reply to Lucas Gass (lukeg) from comment #8)
> We need a rebase here now.

Patches were attached in reserve order.
Comment 12 Lucas Gass (lukeg) 2025-10-22 13:44:52 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 13 Pedro Amorim 2025-10-23 09:26:24 UTC
This appears to be causing the following warning when doing yarn js:watch


WARNING in ./koha-tmpl/intranet-tmpl/prog/js/fetch/api-client.js
  ⚠ Module parse warning:
  ╰─▶   ⚠ Critical dependency: the request of a dependency is an expression
           ╭─[226:26]
       224 │         key,
       225 │         createClientProxy(function() {
       226 │             return import(path);
           ·                           ─────
       227 │         })
       228 │     ];
           ╰────
      


Rspack compiled with 1 warning in 1.01 s
Comment 14 Jonathan Druart 2025-10-23 09:58:27 UTC
Created attachment 188316 [details] [review]
Bug 36674: Fix warning

WARNING in ./koha-tmpl/intranet-tmpl/prog/js/fetch/api-client.js
  ⚠ Module parse warning:
  ╰─▶   ⚠ Critical dependency: the request of a dependency is an expression
           ╭─[226:26]
       224 │         key,
       225 │         createClientProxy(function() {
       226 │             return import(path);
           ·                           ─────
       227 │         })
       228 │     ];
           ╰────
Comment 15 Jonathan Druart 2025-10-23 09:59:24 UTC
Is this patch correct, Paul?

https://webpack.js.org/guides/dependency-management/#requirecontext

Seems a bit useless but it removes the warning.
Comment 16 Paul Derscheid 2025-10-23 10:07:35 UTC
Looking.
Comment 17 Paul Derscheid 2025-10-23 10:35:25 UTC
The problem was actually introduced by
> Bug 36674: Remove duplication

The commit tried to DRY up the code by extracting paths into variables, using import(path) instead of import("./patron-api-client.js").

The problem is that webpack/rspack needs to see those literal strings at build time to know which files to bundle - when you use a variable, it can't figure that out until runtime.

The fix is just to revert 3a6fe57c269367a3dfedee0720b0857314c77dbf.
Comment 18 Paul Derscheid 2025-10-23 10:37:30 UTC
I think this was just a problem for the cypress entry point that actually bundles the thing.
Comment 19 Jonathan Druart 2025-10-23 11:02:36 UTC
Created attachment 188326 [details] [review]
Bug 36674: Revert "Bug 36674: Remove duplication"

This reverts commit 3a6fe57c269367a3dfedee0720b0857314c77dbf.
Comment 20 Lucas Gass (lukeg) 2025-10-23 12:26:24 UTC
3a6fe57c269367a3dfedee0720b0857314c77dbf is reverted from main now
Comment 21 Jonathan Druart 2025-11-04 12:57:13 UTC
We broke api-client.ts

Oops...we found an error preparing this test file:                                                                                                                                                                   
                                                                                                                                                                                                                     
  > t/cypress/integration/t/api-client.ts                                                                                                                                                                            
                                                                                                                                                                                                                     
The error was:                                                                                                                                                                                                       
                                                                                                                                                                                                                     
Error: Webpack Compilation Error                                                                                                                                                                                     
Module parse failed: Unexpected token (1:10)                                                                                                                                                                         
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
Comment 22 Paul Derscheid 2025-11-04 13:06:37 UTC
Taking a look.
Comment 23 Paul Derscheid 2025-11-04 13:39:36 UTC
Created attachment 189012 [details] [review]
Bug 36674: (follow-up) Stop emitting Cypress API client source maps

- Use devtool: false to prevent emitting source maps for cypress.
- Add package script to clean built assets, artifacts.
Comment 24 Paul Derscheid 2025-11-04 13:40:15 UTC
Joubu, try the last patch, run yarn js:clean then rebuild, the error should be gone.
Comment 25 Jonathan Druart 2025-11-06 08:14:44 UTC
+    "js:clean": "find koha-tmpl/intranet-tmpl/prog/js/vue/dist -type f -name '*.js*' -delete && find t/cypress/plugins/dist -type f -name '*.js*' -delete",

Why not `rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/* t/cypress/plugins/dist/*`  ?

The reason I am asking is that there is a subdir koha-tmpl/intranet-tmpl/prog/js/vue/dist/admin that will be left empty with the command you suggested.
Comment 26 Paul Derscheid 2025-11-06 10:44:45 UTC
Oh, sorry. I overlooked that.
Comment 27 Paul Derscheid 2025-11-06 10:51:05 UTC
Created attachment 189153 [details] [review]
Bug 36674: (follow-up) Stop emitting Cypress API client source maps

- Use devtool: false to prevent emitting source maps for cypress.
- Add package script to clean built assets, artifacts.
Comment 28 Paul Derscheid 2025-11-06 10:53:04 UTC
I often use find since I think it's more flexible if you want to keep some stuff but in this case it doesn't provide any benefit.
Comment 29 Jonathan Druart 2025-11-13 11:27:11 UTC
Created attachment 189551 [details] [review]
Bug 36674: (follow-up) Stop emitting Cypress API client source maps

- Use devtool: false to prevent emitting source maps for cypress.
- Add package script to clean built assets, artifacts.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>