Bug 37911 - Prototype vue islands within static pages
Summary: Prototype vue islands within static pages
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Paul Derscheid
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 37824
Blocks: 37939 38941 39849
  Show dependency treegraph
 
Reported: 2024-09-12 19:04 UTC by Paul Derscheid
Modified: 2025-05-07 07:25 UTC (History)
9 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 37911: Prototype vue islands within static pages (7.80 KB, patch)
2024-09-12 19:11 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.15 KB, patch)
2024-09-12 19:17 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (25.00 KB, patch)
2024-09-12 23:43 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.67 KB, patch)
2024-09-13 08:41 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.14 KB, patch)
2024-09-13 10:26 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.25 KB, patch)
2024-09-13 11:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (759 bytes, patch)
2024-09-13 13:37 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.10 KB, patch)
2024-09-16 09:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.82 KB, patch)
2024-09-16 12:27 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (774 bytes, patch)
2024-09-17 11:27 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (16.81 KB, patch)
2024-09-18 17:08 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.42 KB, patch)
2024-10-18 11:15 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.45 KB, patch)
2024-10-18 11:35 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: Prototype vue islands within static pages (7.89 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.23 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (25.11 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.75 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.22 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.34 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.10 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.90 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (857 bytes, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (16.90 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.48 KB, patch)
2024-10-18 12:07 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: Prototype vue islands within static pages (7.88 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.22 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (8.29 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.73 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.21 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.32 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.08 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.89 KB, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (841 bytes, patch)
2025-01-23 14:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (16.88 KB, patch)
2025-01-23 14:31 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.46 KB, patch)
2025-01-23 14:31 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Isolate yarn.lock changes (16.05 KB, patch)
2025-01-23 14:34 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Isolate yarn.lock changes (16.11 KB, patch)
2025-01-23 14:43 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: Prototype vue islands within static pages (7.92 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.22 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (8.58 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.73 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.21 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.22 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.09 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.89 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (841 bytes, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (16.82 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.38 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Isolate yarn.lock changes (132.52 KB, patch)
2025-02-18 14:06 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: Prototype vue islands within static pages (7.91 KB, patch)
2025-04-01 10:02 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.22 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (8.57 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.73 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.20 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.21 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.08 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.89 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (839 bytes, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (16.96 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.36 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Isolate yarn.lock changes (132.52 KB, patch)
2025-04-01 10:03 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove webpack.config.js (2.01 KB, patch)
2025-04-01 10:05 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove unneeded include of calendar.inc in intranet-main.tt (961 bytes, patch)
2025-04-01 10:16 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block (1.44 KB, patch)
2025-04-07 13:10 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps (1.36 KB, patch)
2025-04-07 13:10 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use style-, css-loader to preserve compatiblity with Koha's Vue components, e.g. Dialog.vue (1.28 KB, patch)
2025-04-08 09:02 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: Update Makefile.PL to handle generated "vue/dist" files (3.11 KB, patch)
2025-04-11 04:56 UTC, David Cook
Details | Diff | Splinter Review
Bug 37911: Prototype vue islands within static pages (7.99 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.29 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (8.66 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.80 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.28 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.29 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.16 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.96 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (915 bytes, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (17.04 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.45 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Isolate yarn.lock changes (132.60 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove webpack.config.js (2.08 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove unneeded include of calendar.inc in intranet-main.tt (1.01 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block (1.51 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps (1.43 KB, patch)
2025-04-22 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use style-, css-loader to preserve compatiblity with Koha's Vue components, e.g. Dialog.vue (1.36 KB, patch)
2025-04-22 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: Update Makefile.PL to handle generated "vue/dist" files (3.23 KB, patch)
2025-04-22 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37911: Prototype vue islands within static pages (7.99 KB, patch)
2025-04-25 08:28 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props (1.29 KB, patch)
2025-04-25 08:28 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead (8.66 KB, patch)
2025-04-25 08:28 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting (1.80 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names (1.28 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Scan for requested components in caller and lazy load (5.29 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Enable usage of dependencies (6.14 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback (4.96 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production (915 bytes, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Allow for initialisation of islands on demand (17.04 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove example islands for core submission (12.43 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Isolate yarn.lock changes (132.60 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove webpack.config.js (2.08 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Remove unneeded include of calendar.inc in intranet-main.tt (1.01 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block (1.51 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps (1.43 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Use style-, css-loader to preserve compatiblity with Koha's Vue components, e.g. Dialog.vue (1.36 KB, patch)
2025-04-25 08:29 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: Update Makefile.PL to handle generated "vue/dist" files (3.23 KB, patch)
2025-04-25 08:30 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 37911: (follow-up) Reintroduce @koha-community/prettier-plugin-template-toolkit (963 bytes, patch)
2025-04-26 09:31 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 37911: (follow-up) Associated yarn.lock changes (120.96 KB, patch)
2025-04-26 09:31 UTC, Paul Derscheid
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2024-09-12 19:04:44 UTC
This bug is just for easy sharing of some ideas for reactive islands in static pages.
Comment 1 Paul Derscheid 2024-09-12 19:11:01 UTC
Created attachment 171425 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.
Comment 2 Paul Derscheid 2024-09-12 19:17:24 UTC
Created attachment 171426 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props
Comment 3 Paul Derscheid 2024-09-12 23:43:46 UTC
Created attachment 171432 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.
Comment 4 Paul Derscheid 2024-09-12 23:46:53 UTC
Vue ^3.5.x provides the perfect way to do this!
Was too tired to get the dynamic imports right, but if you apply all three patches you'll see what magic could await us :)
Comment 5 Paul Derscheid 2024-09-13 08:41:19 UTC
Created attachment 171439 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting
Comment 6 Matt Blenkinsop 2024-09-13 09:56:15 UTC
This looks great! I think the vue upgrade should 100% be done. When I tried this a while ago the shadowRoot option was exactly what I was missing - couldn't get any core CSS classes to work in the components

Code splitting seems to work fine, I can see separate bundles for multiple islands (I created some extras to test). Its a shame there doesn't seem to be a `defineAsyncCustomElement` method so that we aren't loading all the bundles on each page even if we aren't using some of the islands
Comment 7 Paul Derscheid 2024-09-13 10:23:00 UTC
Thanks for testing it Matt!

There are some problems currently:
- We need to define a way to provide the dependencies for the preexisting shared components (maybe you'd like to experiment on that one).
- As you pointed out, we need to lazy load what's needed (currently on that one).
- The chunk names are annoying (patch incoming).
Comment 8 Paul Derscheid 2024-09-13 10:26:17 UTC
Created attachment 171440 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names
Comment 9 Paul Derscheid 2024-09-13 11:39:35 UTC
Created attachment 171445 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss
Comment 10 Paul Derscheid 2024-09-13 11:46:30 UTC
Typo, should say hello-islands.js, not .ts.
Comment 11 Paul Derscheid 2024-09-13 13:37:50 UTC
Created attachment 171459 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
Comment 12 Matt Blenkinsop 2024-09-16 09:29:18 UTC
Created attachment 171517 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies
Comment 13 Paul Derscheid 2024-09-16 12:27:50 UTC
Created attachment 171538 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.
Comment 14 Paul Derscheid 2024-09-17 11:27:21 UTC
Created attachment 171608 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
Comment 15 Paul Derscheid 2024-09-18 17:08:13 UTC
Created attachment 171685 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.
Comment 16 Matt Blenkinsop 2024-10-18 11:15:11 UTC
Created attachment 172962 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission
Comment 17 Paul Derscheid 2024-10-18 11:35:51 UTC
Created attachment 172963 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 18 Matt Blenkinsop 2024-10-18 12:07:04 UTC
Created attachment 172965 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 19 Matt Blenkinsop 2024-10-18 12:07:07 UTC
Created attachment 172966 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 20 Matt Blenkinsop 2024-10-18 12:07:10 UTC
Created attachment 172967 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 21 Matt Blenkinsop 2024-10-18 12:07:13 UTC
Created attachment 172968 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 22 Matt Blenkinsop 2024-10-18 12:07:16 UTC
Created attachment 172969 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 23 Matt Blenkinsop 2024-10-18 12:07:19 UTC
Created attachment 172970 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 24 Matt Blenkinsop 2024-10-18 12:07:22 UTC
Created attachment 172971 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies
Comment 25 Matt Blenkinsop 2024-10-18 12:07:25 UTC
Created attachment 172972 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 26 Matt Blenkinsop 2024-10-18 12:07:28 UTC
Created attachment 172973 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 27 Matt Blenkinsop 2024-10-18 12:07:31 UTC
Created attachment 172974 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 28 Matt Blenkinsop 2024-10-18 12:07:33 UTC
Created attachment 172975 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 29 Jonathan Druart 2024-10-21 10:04:39 UTC
Are you/we going to use it? I don't think we should include it if there are no follow-up plans.
Comment 30 Matt Blenkinsop 2024-10-21 10:11:11 UTC
I have a prototype in bug 38010 - it allows the acquisitions left menu to be migrated to Vue and used in a Vue app as well as in template files where required
Comment 31 David Cook 2024-12-16 23:42:51 UTC
Applying: Bug 37911: Prototype vue islands within static pages
Using index info to reconstruct a base tree...
M       koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt
M       rspack.config.js
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 37911: Prototype vue islands within static pages
Comment 32 David Cook 2024-12-16 23:44:58 UTC
I'm trying to fix as I go, but I keep getting more and more conflicts. Please rebase and squash where possible.
Comment 33 David Cook 2024-12-17 00:02:03 UTC
(In reply to Jonathan Druart from comment #29)
> Are you/we going to use it? I don't think we should include it if there are
> no follow-up plans.

It's too difficult to review using Splinter Review and I can't apply the patches, so I can't say for sure yet....but for a long time I've been asking us to bundle the Vue.js differently, so that we can use Vue.js for embedded web components to embed Vue components throughout other parts of Koha and in Koha plugins.

I'd love for everyone to be across the Vue.js work, and for us to start moving ahead further with it.
Comment 34 Matt Blenkinsop 2025-01-23 14:30:31 UTC
Created attachment 176959 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 35 Matt Blenkinsop 2025-01-23 14:30:35 UTC
Created attachment 176960 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 36 Matt Blenkinsop 2025-01-23 14:30:38 UTC
Created attachment 176961 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 37 Matt Blenkinsop 2025-01-23 14:30:42 UTC
Created attachment 176962 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 38 Matt Blenkinsop 2025-01-23 14:30:45 UTC
Created attachment 176963 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 39 Matt Blenkinsop 2025-01-23 14:30:49 UTC
Created attachment 176964 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 40 Matt Blenkinsop 2025-01-23 14:30:52 UTC
Created attachment 176965 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies
Comment 41 Matt Blenkinsop 2025-01-23 14:30:56 UTC
Created attachment 176966 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 42 Matt Blenkinsop 2025-01-23 14:30:59 UTC
Created attachment 176967 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 43 Matt Blenkinsop 2025-01-23 14:31:03 UTC
Created attachment 176968 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 44 Matt Blenkinsop 2025-01-23 14:31:07 UTC
Created attachment 176969 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 45 Matt Blenkinsop 2025-01-23 14:34:09 UTC
Created attachment 176970 [details] [review]
Bug 37911: (follow-up) Isolate yarn.lock changes
Comment 46 Matt Blenkinsop 2025-01-23 14:35:24 UTC
Looks like the yarn.lock file was causing conflicts due to version changes since this was uploaded. I've isolated it into its own commit to make rebasing easier and allow the RM to decide whether to push that commit or drop it and generate a fresh lock file at the time of pushing
Comment 47 Paul Derscheid 2025-01-23 14:43:06 UTC
Created attachment 176972 [details] [review]
Bug 37911: (follow-up) Isolate yarn.lock changes

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 48 Matt Blenkinsop 2025-02-18 14:06:05 UTC
Created attachment 178256 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 49 Matt Blenkinsop 2025-02-18 14:06:08 UTC
Created attachment 178257 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 50 Matt Blenkinsop 2025-02-18 14:06:11 UTC
Created attachment 178258 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 51 Matt Blenkinsop 2025-02-18 14:06:14 UTC
Created attachment 178259 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 52 Matt Blenkinsop 2025-02-18 14:06:17 UTC
Created attachment 178260 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 53 Matt Blenkinsop 2025-02-18 14:06:20 UTC
Created attachment 178261 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 54 Matt Blenkinsop 2025-02-18 14:06:23 UTC
Created attachment 178262 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies
Comment 55 Matt Blenkinsop 2025-02-18 14:06:25 UTC
Created attachment 178263 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 56 Matt Blenkinsop 2025-02-18 14:06:28 UTC
Created attachment 178264 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 57 Matt Blenkinsop 2025-02-18 14:06:31 UTC
Created attachment 178265 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 58 Matt Blenkinsop 2025-02-18 14:06:34 UTC
Created attachment 178266 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 59 Matt Blenkinsop 2025-02-18 14:06:37 UTC
Created attachment 178267 [details] [review]
Bug 37911: (follow-up) Isolate yarn.lock changes

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 60 Matt Blenkinsop 2025-02-18 14:07:23 UTC
Rebased to include the new tidy work
Comment 61 Marcel de Rooy 2025-03-07 10:39:17 UTC
 FAIL
   FAIL   git manipulation
                The file has been added and deleted in the same patchset
Comment 62 Martin Renvoize (ashimema) 2025-03-11 16:28:43 UTC
(In reply to Marcel de Rooy from comment #61)
>  FAIL
>    FAIL   git manipulation
>                 The file has been added and deleted in the same patchset

I think that's being thrown due to this commit: "Remove example islands for core submission"

However, personally I think it's a helpful piece of history to keep.. it gives an example to future developers of how the code should be used...

That said.. we could squash that one and document the examples elsewhere.. I think we should have both really... but I also don't think we should hold this one back
Comment 63 Paul Derscheid 2025-03-11 16:32:59 UTC
Can't we paste it into the wiki into the soon to be vue islands docs (;D) and squash the commit?
Comment 64 Jonathan Druart 2025-03-12 08:35:41 UTC
If we have a commit id (from the PTFS-e repo) we can cherry-pick to get the example applied on current main that would be enough IMO.
Comment 65 Tomás Cohen Arazi (tcohen) 2025-03-12 12:26:54 UTC
Can we focus on the feature, and then when this is PQA and we are happy with it, we just remove the sample stuff? Seems like an easy direction to take.
Comment 66 Jonathan Druart 2025-03-12 13:49:36 UTC
Reviewed a squashed version:
1. This is wrong
-    "@babel/core": "^7.26.8",
-    "@koha-community/prettier-plugin-template-toolkit": "^2.0.2",

2. This line should be removed
+    [% INCLUDE 'calendar.inc' %]

3. We should either keep webpack.config.js in sync with rspack or remove it

4. dist files are listed in Makefile.PL

5. That's a lot of duplicated code in rspack.config.js, cannot we improve that?
Comment 67 Paul Derscheid 2025-03-12 14:46:46 UTC
That's even better: comment #64

To comment #66:
3) Agree that we should either keep rspack and webpack configs in sync or remove the webpack config.

I think the removal is probably better to lower maintenance.


5) That's true, but I think we should wait a bit before we abstract to see how the config evolves.
Comment 68 David Cook 2025-03-26 04:27:32 UTC
Really looking forward to this change :)
Comment 69 Paul Derscheid 2025-04-01 10:02:58 UTC
Created attachment 180113 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 70 Paul Derscheid 2025-04-01 10:03:01 UTC
Created attachment 180114 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 71 Paul Derscheid 2025-04-01 10:03:05 UTC
Created attachment 180115 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 72 Paul Derscheid 2025-04-01 10:03:08 UTC
Created attachment 180116 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 73 Paul Derscheid 2025-04-01 10:03:12 UTC
Created attachment 180117 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 74 Paul Derscheid 2025-04-01 10:03:15 UTC
Created attachment 180118 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 75 Paul Derscheid 2025-04-01 10:03:18 UTC
Created attachment 180119 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies
Comment 76 Paul Derscheid 2025-04-01 10:03:22 UTC
Created attachment 180120 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 77 Paul Derscheid 2025-04-01 10:03:26 UTC
Created attachment 180121 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 78 Paul Derscheid 2025-04-01 10:03:29 UTC
Created attachment 180122 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 79 Paul Derscheid 2025-04-01 10:03:33 UTC
Created attachment 180123 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 80 Paul Derscheid 2025-04-01 10:03:37 UTC
Created attachment 180124 [details] [review]
Bug 37911: (follow-up) Isolate yarn.lock changes

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 81 Paul Derscheid 2025-04-01 10:04:21 UTC
Rebased
Comment 82 Paul Derscheid 2025-04-01 10:05:16 UTC
Created attachment 180125 [details] [review]
Bug 37911: (follow-up) Remove webpack.config.js
Comment 83 Paul Derscheid 2025-04-01 10:16:37 UTC
Created attachment 180129 [details] [review]
Bug 37911: (follow-up) Remove unneeded include of calendar.inc in intranet-main.tt
Comment 84 Jonathan Druart 2025-04-07 12:27:06 UTC
A note about the example: There is a warning in the console when the surprise pops up.

We need this:

-    <dialog-island id="surprise"></dialog-island>
+    <dialog-island></dialog-island>

-                $("#surprise").attr("warning", `<span>Yes, this is <span class="font-monospace">Dialog.vue</span> (in a wrapper component)!</span><span class="d-none">${Math.random()}</span>`);
+                $("dialog-island").attr("warning", `<span>Yes, this is <span class="font-monospace">Dialog.vue</span> (in a wrapper component)!</span><span class="d-none">${Math.random()}</span>`);
Comment 85 Paul Derscheid 2025-04-07 12:29:17 UTC
I think the last problem we currently have is how to best handle the files that need to be included in Makefile.PL. Ideas?
Comment 86 Paul Derscheid 2025-04-07 12:47:15 UTC
Just noticed that I overlooked something when rebasing.
Comment 87 Paul Derscheid 2025-04-07 13:10:41 UTC
Created attachment 180696 [details] [review]
Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block
Comment 88 Paul Derscheid 2025-04-07 13:10:44 UTC
Created attachment 180697 [details] [review]
Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps

- Remove 'experiments: css' for the first config block since it seems to do 2 passes when building the css.
- Remove style-, css-loader configuration for islands as I assume it is unneeded in this case.
Comment 89 Jonathan Druart 2025-04-07 14:33:14 UTC
(In reply to Paul Derscheid from comment #85)
> I think the last problem we currently have is how to best handle the files
> that need to be included in Makefile.PL. Ideas?

It's a hard one, or I am missing something.

I don't manage to make the list "dynamic" (pass a * for instance).
Comment 90 Paul Derscheid 2025-04-07 14:42:24 UTC
I think one way to do it is to sort of include the generated files in dist via mapping in a separate file. 

The question is just whether ExtUtils::MakeMaker supports that and whether we have to make many changes to Makefile.PL to support that?

Would maybe be useful in other contexts as well, though.
Comment 91 Jonathan Druart 2025-04-08 07:32:51 UTC Comment hidden (obsolete)
Comment 92 Jonathan Druart 2025-04-08 07:34:18 UTC Comment hidden (obsolete)
Comment 93 Jonathan Druart 2025-04-08 07:55:28 UTC
1. Todos
IMO to make this ready we need to:
 a. Attach to this bug the patches that are about the islands concept
 b. Move to a branch the patches for the example
 b. Remove [contenthash] from the filenames

2. Question - Also, can you explain why do we need both .esm.js and .js files?

3. Confirm bug?
There is something wrong I think, somewhere...
I tried to test removing the contenthash and see which files were generated, on top of the example but with the last patches, and I get:

ERROR in ./koha-tmpl/intranet-tmpl/prog/js/vue/components/Dialog.vue?vue&type=style&index=0&id=060b00a6&scoped=true&lang=css (../../../../../../node_modules/vue-loader/dist/index.js??ruleSet[0]!./koha-tmpl/intranet-tmpl/prog/js/vue/components/Dialog.vue?vue&type=style&index=0&id=060b00a6&scoped=true&lang=css)
  × Module parse failed:
  ╰─▶   × JavaScript parsing error: Expression expected
         ╭─[2:0]
       1 │
       2 │ .modal {
         · ─
       3 │     position: fixed;
       4 │     z-index: 9998;
         ╰────

The git log is:
2fce1bbd165 (HEAD) Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps
f0325f3235a Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block
472e8503bc9 Bug 37911: (follow-up) Remove webpack.config.js
f0afb0a3e41 Bug 37911: (follow-up) Isolate yarn.lock changes
396442e8d13 Bug 37911: (follow-up) Allow for initialisation of islands on demand
c10525beedb Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
bcd6b19c98f Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback
7b8bd7fec9b Bug 37911: (follow-up) Enable usage of dependencies
561ff807fcf Bug 37911: (follow-up) Scan for requested components in caller and lazy load
78e159d9001 Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names
6732a1c5a20 Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting
148c67f284a Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead
0f0779ace1d Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props
2c9b8c5925e Bug 37911: Prototype vue islands within static pages
ef4d0aa5c58 (origin/main) Bug 39460: Fix koha-upgrade-schema when no DB change needed
Comment 94 Jonathan Druart 2025-04-08 08:00:58 UTC
(In reply to Jonathan Druart from comment #93)
> 1. Todos
> IMO to make this ready we need to:
>  a. Attach to this bug the patches that are about the islands concept
>  b. Move to a branch the patches for the example
>  c. Remove [contenthash] from the filenames

(it was c)

If c. is possible and works, then
 d. add the filenames to Makefile.PL
Comment 95 Paul Derscheid 2025-04-08 08:38:11 UTC
Ah ok, so the error in comment #93 means that we do need to use style-, css-loader to preserve compatibility with the Koha Vue components. 

Wasn't sure, but that seems to confirm it.
Comment 96 Paul Derscheid 2025-04-08 08:41:16 UTC
The js vs esm are for different purposes, the regular js files are for:
> - Instead check the "init" attribute for the currentScript when using the umd.
To get an IIFE that immediately is accessible like a normal script.
> - Allow for manual import and call when using the esm.
For usage with:
<script type="module">
  import { something } from "./something"
...
Comment 97 Paul Derscheid 2025-04-08 09:02:01 UTC
Created attachment 180714 [details] [review]
Bug 37911: (follow-up) Use style-, css-loader to preserve compatiblity with Koha's Vue components, e.g. Dialog.vue

- For now let's not rely on rspack's native css loader to parse and
  bundle styles as we need to preserve compatibility.
- Reintroduce configuration block for style-, css-loader for islands.
Comment 98 Paul Derscheid 2025-04-08 09:02:35 UTC
The last patch should fix the error you were encountering Jonathan :)
Comment 99 David Cook 2025-04-09 00:35:10 UTC
(In reply to Paul Derscheid from comment #90)
> I think one way to do it is to sort of include the generated files in dist
> via mapping in a separate file. 
> 
> The question is just whether ExtUtils::MakeMaker supports that and whether
> we have to make many changes to Makefile.PL to support that?
> 
> Would maybe be useful in other contexts as well, though.

I haven't been following this bug closely, but if you need any help with ExtUtils::MakeMaker, I'm happy to help. I've had to learn some of its intricacies over the years. 

If you look at "sub top_targets" in Makefile.PL you can see some work I've done in the past. (Not that I'm 100% happy with the Makefile text I output there. I can't remember why I used the all target, but I imagine I had my reasons and it works well enough...)
Comment 100 David Cook 2025-04-09 00:36:26 UTC
(In reply to Paul Derscheid from comment #85)
> I think the last problem we currently have is how to best handle the files
> that need to be included in Makefile.PL. Ideas?

As per my previous comment, if you can give me the precise problem, I'll do my best to help :). April is a crazy month for me but I'll try my best.
Comment 101 Jonathan Druart 2025-04-09 07:24:27 UTC
(In reply to David Cook from comment #100)
> (In reply to Paul Derscheid from comment #85)
> > I think the last problem we currently have is how to best handle the files
> > that need to be included in Makefile.PL. Ideas?
> 
> As per my previous comment, if you can give me the precise problem, I'll do
> my best to help :). April is a crazy month for me but I'll try my best.

I am not very familiar with this script or how it's supposed to be used. But from what I understand (please correct me if I'm wrong), we're supposed to follow these three steps:

  1. perl Makefile.PL
  2. make
  3. sudo make install

During the first step, we list the different files ($file_map), then we build the resource (build-resource.PL), and finally, we copy the files.

The issue is that we are now generating multiple .js files into dists/, but they don't exist yet during step 1.

One possible (though not ideal) solution could be to run build-resource.PL before generating the file map and inject `ls dists/*.js` into it.

By the way, I'm not sure why both steps 2 and 3 call build-resource.PL.
Comment 102 David Cook 2025-04-10 00:57:36 UTC
(In reply to Jonathan Druart from comment #101)
> I am not very familiar with this script or how it's supposed to be used. But
> from what I understand (please correct me if I'm wrong), we're supposed to
> follow these three steps:
> 
>   1. perl Makefile.PL
>   2. make
>   3. sudo make install
> 
> During the first step, we list the different files ($file_map), then we
> build the resource (build-resource.PL), and finally, we copy the files.
> 
> The issue is that we are now generating multiple .js files into dists/, but
> they don't exist yet during step 1.
> 
> One possible (though not ideal) solution could be to run build-resource.PL
> before generating the file map and inject `ls dists/*.js` into it.

I'm not sure that I understand. 

Currently, we add the names of generated .js files to Makefile.PL (e.g. koha-tmpl/intranet-tmpl/prog/js/vue/dist/erm.js) so that it can include them in the Makefile that is generated by "perl Makefile.PL", so that "make" knows what to build and what "make install" knows what to install. (Otherwise, it will complain that there are files there that it doesn't know about and fails to build.)

Is the problem that we don't know the names of the generated files, so we can't pre-add them to Makefile.PL?

Honestly, I think that there are make rules that we can add for dynamically generated files, so that we don't have to list them in Makefile.PL anymore... 

Some experimenting to do there... 
 
> By the way, I'm not sure why both steps 2 and 3 call build-resource.PL.

So step 1 just makes the makefile. 

Step 2 "make" then runs builds the default target which is "makemakerdflt". "makemakedflt" builds the "all" target.

The "all" target builds "compile_css_js", "pure_all", "manify_pods". 

build-resource.PL is run when "compile_css_js" is built.

Note: I am the person who added compile_css_js to "all". I can't recall why exactly. It's something I've been meaning to revisit as it seems inefficient/unnecessary to do it twice...

Step 3 calls the "install target" which for some reason I don't know which dates back to at least 2007 also includes the "all" target, so that all gets built again.

We can certainly look at improving this.
Comment 103 David Cook 2025-04-10 00:58:46 UTC
If someone shares an error message with me or gives me the instructions to get to the problem point, I can take a look at fixing it. 

I've got ideas >_>
Comment 104 Jonathan Druart 2025-04-10 07:36:18 UTC
(In reply to David Cook from comment #102)
> (In reply to Jonathan Druart from comment #101)
> I'm not sure that I understand. 

I think you did, you rephrase what I said (or at least tried to say) in a more verbose way :D

(In reply to David Cook from comment #103)
> If someone shares an error message with me or gives me the instructions to
> get to the problem point, I can take a look at fixing it. 

There is no error message.
Just try:
0. rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/*
1. apply the patches, yarn install && yarn build
2. Notice that more than admin, preservation.js and erm.js have been generated in dist
3. Generate the makefile, make, make install and notice that only admin, erm.js, preservation.js have been copied to /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/js/vue/dist/

Expected: All the .js file should be copied.

Note that I don't think we should copy the .esm.js files.
Comment 105 David Cook 2025-04-11 03:16:57 UTC
(In reply to Jonathan Druart from comment #104)
> There is no error message.
> Just try:
> 0. rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/*
> 1. apply the patches, yarn install && yarn build
> 2. Notice that more than admin, preservation.js and erm.js have been
> generated in dist
> 3. Generate the makefile, make, make install and notice that only admin,
> erm.js, preservation.js have been copied to
> /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/js/vue/dist/
> 
> Expected: All the .js file should be copied.

Interesting. I thought that make should complain about unknown files being there. Maybe I had it backwards and it only complains if known files are missing. 

Ok, I've got something in mind for this. I shouldn't work on it today, but I might take a few minutes...
 
> Note that I don't think we should copy the .esm.js files.

While I haven't looked at the files yet, in theory I think we should copy the .esm.js files, as I'm quite interested in working with them in the future. I find it very nice working with ESM, especially for things like plugins or *UserJS.
Comment 106 David Cook 2025-04-11 03:34:58 UTC
I tried doing "make install" as kohadev-koha and root but they both ran into issues when using the default config during "perl Makefile.PL", so I'll do an alternate test...
Comment 107 David Cook 2025-04-11 03:51:36 UTC
(In reply to Jonathan Druart from comment #104)
> Just try:
> 0. rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/*
> 1. apply the patches, yarn install && yarn build
> 2. Notice that more than admin, preservation.js and erm.js have been
> generated in dist
> 3. Generate the makefile, make, make install and notice that only admin,
> erm.js, preservation.js have been copied to
> /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/js/vue/dist/
> 
> Expected: All the .js file should be copied.

So this procedure threw me into a bit of confusing, because I did not have the same experience with Step 3. It actually worked fine for me in Step 3 (at least when using the "single" installation method. But I'll explain below...

If you run the yarn build BEFORE perl Makefile.PL, it appears that the Makefile.PL automagically finds islands.esm.js and adds it into all the right places in the Makefile. (Probably because of the "target_map".) Then when you do "make" and "make install", islands.esm.js gets installed into the deployment directory.

*However*

If you DO NOT run yarn build BEFORE perl Makefile.PL, the Makefile won't know about islands.esm.js and thus doesn't copy it into blib and doesn't install it.
Comment 108 David Cook 2025-04-11 03:57:49 UTC
If I recall correctly, "dh_auto_install" in debian/rules has a Perl handler which automatically runs "perl Makefile.PL", "make", and "make install", and since the Debian packaging runs against a git checkout source tarball, there's no running "perl build-resources.PL" ahead of time. 

Plus, I recall we wanted to make it so that all installation methods worked the same way. 

In hindsight, I guess we could've always shelled out from Makefile.PL to do build-resources.PL before running "get_file_map()" in Makefile.PL.
Comment 109 David Cook 2025-04-11 04:50:54 UTC
So I've realized that Koha really doesn't need "make" and "make install".

We thought "build-resources.PL" was being repeated, but in reality *everything* done by "make" is done again in "make install" anyway.
Comment 110 David Cook 2025-04-11 04:56:46 UTC
Created attachment 180805 [details] [review]
Bug 37911: Update Makefile.PL to handle generated "vue/dist" files

This change modifies Makefile.PL so that we no longer have to
manually add to the Makefile.PL all the files that end up in the
"vue/dist" folder.

Instead, we dynamically generate the files, and then copy them over
to the build target.

Test plan:
0. Apply all patches
1. Setup intall target
sudo mkdir /opt/koha
sudo chown kohadev-koha  /opt/koha
2. Clean build env
rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/* blib Makefile /opt/koha/*
3. perl Makefile.PL
Installation mode (dev, single, standard) [standard] single
Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha
NOTE: Otherwise accept all defaults
4. make install
5. "vue/dist" successfully installed
Note all our expected files are installed including islands.esm.js
Comment 111 David Cook 2025-04-11 04:59:34 UTC
Feel free to obsolete my patch, if I misunderstood the goal.

It would've been easy enough to just add islands.esm.js to the file map like we've done with erm.js, preservation.js, and admin/record_sources.js. 

But this provides a totally dynamic way of managing "vue/js". 

Note that it copies *everything*, so that will include the *.js.map as well. Feel free to update that as needed.

Hopefully the code and code comments make it clear what I'm doing there.
Comment 112 Jonathan Druart 2025-04-22 14:29:02 UTC
Created attachment 181269 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 113 Jonathan Druart 2025-04-22 14:29:06 UTC
Created attachment 181270 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 114 Jonathan Druart 2025-04-22 14:29:10 UTC
Created attachment 181271 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 115 Jonathan Druart 2025-04-22 14:29:13 UTC
Created attachment 181272 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 116 Jonathan Druart 2025-04-22 14:29:17 UTC
Created attachment 181273 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 117 Jonathan Druart 2025-04-22 14:29:21 UTC
Created attachment 181274 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 118 Jonathan Druart 2025-04-22 14:29:24 UTC
Created attachment 181275 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 119 Jonathan Druart 2025-04-22 14:29:28 UTC
Created attachment 181276 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 120 Jonathan Druart 2025-04-22 14:29:32 UTC
Created attachment 181277 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 121 Jonathan Druart 2025-04-22 14:29:36 UTC
Created attachment 181278 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 122 Jonathan Druart 2025-04-22 14:29:40 UTC
Created attachment 181279 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 123 Jonathan Druart 2025-04-22 14:29:43 UTC
Created attachment 181280 [details] [review]
Bug 37911: (follow-up) Isolate yarn.lock changes

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 124 Jonathan Druart 2025-04-22 14:29:47 UTC
Created attachment 181281 [details] [review]
Bug 37911: (follow-up) Remove webpack.config.js

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 125 Jonathan Druart 2025-04-22 14:29:51 UTC
Created attachment 181282 [details] [review]
Bug 37911: (follow-up) Remove unneeded include of calendar.inc in intranet-main.tt

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 126 Jonathan Druart 2025-04-22 14:29:54 UTC
Created attachment 181283 [details] [review]
Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 127 Jonathan Druart 2025-04-22 14:29:58 UTC
Created attachment 181284 [details] [review]
Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps

- Remove 'experiments: css' for the first config block since it seems to do 2 passes when building the css.
- Remove style-, css-loader configuration for islands as I assume it is unneeded in this case.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 128 Jonathan Druart 2025-04-22 14:30:02 UTC
Created attachment 181285 [details] [review]
Bug 37911: (follow-up) Use style-, css-loader to preserve compatiblity with Koha's Vue components, e.g. Dialog.vue

- For now let's not rely on rspack's native css loader to parse and
  bundle styles as we need to preserve compatibility.
- Reintroduce configuration block for style-, css-loader for islands.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 129 Jonathan Druart 2025-04-22 14:30:05 UTC
Created attachment 181286 [details] [review]
Bug 37911: Update Makefile.PL to handle generated "vue/dist" files

This change modifies Makefile.PL so that we no longer have to
manually add to the Makefile.PL all the files that end up in the
"vue/dist" folder.

Instead, we dynamically generate the files, and then copy them over
to the build target.

Test plan:
0. Apply all patches
1. Setup intall target
sudo mkdir /opt/koha
sudo chown kohadev-koha  /opt/koha
2. Clean build env
rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/* blib Makefile /opt/koha/*
3. perl Makefile.PL
Installation mode (dev, single, standard) [standard] single
Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha
NOTE: Otherwise accept all defaults
4. make install
5. "vue/dist" successfully installed
Note all our expected files are installed including islands.esm.js

Amended-by: Jonathan Druart
Tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 130 Jonathan Druart 2025-04-22 14:30:26 UTC
(In reply to David Cook from comment #111)
> Hopefully the code and code comments make it clear what I'm doing there.

Excellent, thanks a lot for your help here!
Comment 131 Jonathan Druart 2025-04-22 14:30:58 UTC
(In reply to Jonathan Druart from comment #93)
> 1. Todos
> IMO to make this ready we need to:
>  a. Attach to this bug the patches that are about the islands concept
>  b. Move to a branch the patches for the example

Not addressed, but not blocker...
Comment 132 David Cook 2025-04-22 23:22:47 UTC
(In reply to Jonathan Druart from comment #130)
> (In reply to David Cook from comment #111)
> > Hopefully the code and code comments make it clear what I'm doing there.
> 
> Excellent, thanks a lot for your help here!

Thanks for the thanks! It was an interesting challenge!
Comment 133 Katrin Fischer 2025-04-25 06:43:04 UTC
Not trusting my self with Vue, can you please help and rebase ASAP?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 37911: Prototype vue islands within static pages
Applying: Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props
Applying: Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead
Applying: Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting
Applying: Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names
Applying: Bug 37911: (follow-up) Scan for requested components in caller and lazy load
Applying: Bug 37911: (follow-up) Enable usage of dependencies
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/js/vue/stores/main.js
M	koha-tmpl/intranet-tmpl/prog/js/vue/stores/navigation.js
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/js/vue/stores/navigation.js
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/vue/stores/navigation.js
Auto-merging koha-tmpl/intranet-tmpl/prog/js/vue/stores/main.js
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/vue/stores/main.js
error: Failed to merge in the changes.
Patch failed at 0001 Bug 37911: (follow-up) Enable usage of dependencies
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-37911-follow-up-Enable-usage-of-dependencies-dwhd9503.patch
Comment 134 Matt Blenkinsop 2025-04-25 08:23:37 UTC
Having a look, think it will be bug 37930 conflicting
Comment 135 Matt Blenkinsop 2025-04-25 08:28:48 UTC
Created attachment 181473 [details] [review]
Bug 37911: Prototype vue islands within static pages

This is pretty bare bones, but might still be interesting to take a look at.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 136 Matt Blenkinsop 2025-04-25 08:28:51 UTC
Created attachment 181474 [details] [review]
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 137 Matt Blenkinsop 2025-04-25 08:28:57 UTC
Created attachment 181475 [details] [review]
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead

- This would require us to update vue to a new minor version.
	- We could then use the defineCustomElement function with the shadowRoot option set to false (this is important).
- The current implementation in islands.ts is naive and would result in an intolerable bundle size.
	- The right way to do code splitting here is to use dynamic imports.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 138 Matt Blenkinsop 2025-04-25 08:29:02 UTC
Created attachment 181476 [details] [review]
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 139 Matt Blenkinsop 2025-04-25 08:29:07 UTC
Created attachment 181477 [details] [review]
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 140 Matt Blenkinsop 2025-04-25 08:29:13 UTC
Created attachment 181478 [details] [review]
Bug 37911: (follow-up) Scan for requested components in caller and lazy load

This should ensure that we only load what's requested.
Maybe we can also find a way to call the exported hydrate function on demand.
To test:
1) Apply the patch
2) Run yarn js:watch and wait for the first build if necessary
3) Load mainpage.pl and note that only hello-islands.ts is imported.
4) Uncomment <hello-world></hello-world> in intranet-main.tt
5) Reload the page
6) Note that now both component chunks are imported on demand
7) Discuss

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 141 Matt Blenkinsop 2025-04-25 08:29:18 UTC
Created attachment 181479 [details] [review]
Bug 37911: (follow-up) Enable usage of dependencies

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 142 Matt Blenkinsop 2025-04-25 08:29:23 UTC
Created attachment 181480 [details] [review]
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback

- Additionally:
    - Use a type instead of an interface.
        - Add JSDoc to it.
    - Use optional chaining when checking for existence and length of config.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 143 Matt Blenkinsop 2025-04-25 08:29:27 UTC
Created attachment 181481 [details] [review]
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 144 Matt Blenkinsop 2025-04-25 08:29:30 UTC
Created attachment 181482 [details] [review]
Bug 37911: (follow-up) Allow for initialisation of islands on demand

- Split the rspack config into a umd and an esm section.
- Don't hydrate by default.
    - Instead check the "init" attribute for the currentScript when using the umd.
    - Allow for manual import and call when using the esm.
    - Add examples for both in intranet-main.tt (to be removed).
- Add a DialogIsland wrapper for the shared Dialog component (to map props to stored refs).
- Move DialogIsland as well as Demo components into an Islands directory under components.
- Add i18n initialisation to hydrate's idle callback.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 145 Matt Blenkinsop 2025-04-25 08:29:34 UTC
Created attachment 181483 [details] [review]
Bug 37911: (follow-up) Remove example islands for core submission

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 146 Matt Blenkinsop 2025-04-25 08:29:38 UTC
Created attachment 181484 [details] [review]
Bug 37911: (follow-up) Isolate yarn.lock changes

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 147 Matt Blenkinsop 2025-04-25 08:29:42 UTC
Created attachment 181485 [details] [review]
Bug 37911: (follow-up) Remove webpack.config.js

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 148 Matt Blenkinsop 2025-04-25 08:29:46 UTC
Created attachment 181486 [details] [review]
Bug 37911: (follow-up) Remove unneeded include of calendar.inc in intranet-main.tt

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 149 Matt Blenkinsop 2025-04-25 08:29:49 UTC
Created attachment 181487 [details] [review]
Bug 37911: (follow-up) Tidy and remove leftover entry point for islands in first config block

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 150 Matt Blenkinsop 2025-04-25 08:29:54 UTC
Created attachment 181488 [details] [review]
Bug 37911: (follow-up) Let style-, css-loader handle styles in Koha's Vue apps

- Remove 'experiments: css' for the first config block since it seems to do 2 passes when building the css.
- Remove style-, css-loader configuration for islands as I assume it is unneeded in this case.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 151 Matt Blenkinsop 2025-04-25 08:29:57 UTC
Created attachment 181489 [details] [review]
Bug 37911: (follow-up) Use style-, css-loader to preserve compatiblity with Koha's Vue components, e.g. Dialog.vue

- For now let's not rely on rspack's native css loader to parse and
  bundle styles as we need to preserve compatibility.
- Reintroduce configuration block for style-, css-loader for islands.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 152 Matt Blenkinsop 2025-04-25 08:30:01 UTC
Created attachment 181490 [details] [review]
Bug 37911: Update Makefile.PL to handle generated "vue/dist" files

This change modifies Makefile.PL so that we no longer have to
manually add to the Makefile.PL all the files that end up in the
"vue/dist" folder.

Instead, we dynamically generate the files, and then copy them over
to the build target.

Test plan:
0. Apply all patches
1. Setup intall target
sudo mkdir /opt/koha
sudo chown kohadev-koha  /opt/koha
2. Clean build env
rm -rf koha-tmpl/intranet-tmpl/prog/js/vue/dist/* blib Makefile /opt/koha/*
3. perl Makefile.PL
Installation mode (dev, single, standard) [standard] single
Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha
NOTE: Otherwise accept all defaults
4. make install
5. "vue/dist" successfully installed
Note all our expected files are installed including islands.esm.js

Amended-by: Jonathan Druart
Tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 153 Katrin Fischer 2025-04-25 16:36:20 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 154 Katrin Fischer 2025-04-26 08:56:50 UTC
i am not sure if it's caused by this, but something with the prettier seems wrong - could it be due to the yarn.lock changes here?
Comment 155 Paul Derscheid 2025-04-26 08:59:17 UTC
What behaviour changes have you found?
Comment 156 Katrin Fischer 2025-04-26 09:04:38 UTC
(In reply to Paul Derscheid from comment #155)
> What behaviour changes have you found?

Look at Jenkins... and the results from testing David has noted on bug 39677.
Comment 157 Paul Derscheid 2025-04-26 09:18:55 UTC
Yeah, I think the prettier plugin was removed from package.json by accident during the rebase in  'Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead'.
Comment 158 Paul Derscheid 2025-04-26 09:31:16 UTC
Created attachment 181566 [details] [review]
Bug 37911: (follow-up) Reintroduce @koha-community/prettier-plugin-template-toolkit

- This includes its dependency @babel/core.
Comment 159 Paul Derscheid 2025-04-26 09:31:20 UTC
Created attachment 181567 [details] [review]
Bug 37911: (follow-up) Associated yarn.lock changes
Comment 160 Paul Derscheid 2025-04-26 09:32:20 UTC
I think those last two patches should fix all the failing tests.
Comment 161 Katrin Fischer 2025-04-26 09:38:36 UTC
Thanks for the quick work Paul, I will push first thing Monday morning.
Comment 162 David Cook 2025-05-07 07:25:49 UTC
Looks like I made a mistake with that Makefile.PL patch.

I've added the fix on bug 39849.