Bug 37911

Summary: Prototype vue islands within static pages
Product: Koha Reporter: Paul Derscheid <paul.derscheid>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: blawlor, dcook, jonathan.druart, martin.renvoize, matt.blenkinsop, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 37824    
Bug Blocks: 37939, 38941    
Attachments: Bug 37911: Prototype vue islands within static pages
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names
Bug 37911: (follow-up) Scan for requested components in caller and lazy load
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
Bug 37911: (follow-up) Enable usage of dependencies
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
Bug 37911: (follow-up) Allow for initialisation of islands on demand
Bug 37911: (follow-up) Remove example islands for core submission
Bug 37911: (follow-up) Remove example islands for core submission
Bug 37911: Prototype vue islands within static pages
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names
Bug 37911: (follow-up) Scan for requested components in caller and lazy load
Bug 37911: (follow-up) Enable usage of dependencies
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
Bug 37911: (follow-up) Allow for initialisation of islands on demand
Bug 37911: (follow-up) Remove example islands for core submission
Bug 37911: Prototype vue islands within static pages
Bug 37911: (follow-up) Correct wishful thinking in displayed comment about changes to data-props
Bug 37911: (follow-up) Update to vue ^3.5.4, use vue's web component implementation instead
Bug 37911: (follow-up) Experimental introduction of dynamic imports for code splitting
Bug 37911: (follow-up) Use webpack import annotations for more concise chunk names
Bug 37911: (follow-up) Scan for requested components in caller and lazy load
Bug 37911: (follow-up) Enable usage of dependencies
Bug 37911: (follow-up) Move store initialisation into hydrate's idle callback
Bug 37911: (follow-up) Prevent resolution failure of dynamic imports when building in mode --production
Bug 37911: (follow-up) Allow for initialisation of islands on demand
Bug 37911: (follow-up) Remove example islands for core submission
Bug 37911: (follow-up) Isolate yarn.lock changes
Bug 37911: (follow-up) Isolate yarn.lock changes

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>