Bug 37911 - Prototype vue islands within static pages
Summary: Prototype vue islands within static pages
Status: Failed QA
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: Testopia
URL:
Keywords: Hackfest
Depends on: 37824
Blocks: 37939 38941
  Show dependency treegraph
 
Reported: 2024-09-12 19:04 UTC by Paul Derscheid
Modified: 2025-04-03 11:44 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
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

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