Bug 40150 - Prevent uncaught error on multiple attempts to 'define' on 'CustomElementsRegistry' in islands.ts
Summary: Prevent uncaught error on multiple attempts to 'define' on 'CustomElementsReg...
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Paul Derscheid
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-06-16 14:58 UTC by Paul Derscheid
Modified: 2025-06-16 22:07 UTC (History)
1 user (show)

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


Attachments
[DO NOT PUSH] Bug 40150: Set up a duplicate call to hydrate() (796 bytes, patch)
2025-06-16 15:33 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 40150: Prevent uncaught error on multiple attempts to 'define' on 'CustomElementsRegistry' in islands.ts (1.79 KB, patch)
2025-06-16 15:33 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 40150: Set up a duplicate call to hydrate() (828 bytes, patch)
2025-06-16 22:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 40150: Prevent uncaught error on multiple attempts to 'define' on 'CustomElementsRegistry' in islands.ts (1.83 KB, patch)
2025-06-16 22:01 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2025-06-16 14:58:35 UTC
The CustomElementRegistry does not permit multiple definition attempts on the same name of the custom element in the same context.

This does not lead to an actual bug in behaviour, but it's an uncaught error that should be caught.

Maybe it would also make sense to have a console.debug that warns (if full output is enabled) that this is happening.
Comment 1 Paul Derscheid 2025-06-16 15:33:51 UTC
Created attachment 183287 [details] [review]
[DO NOT PUSH] Bug 40150: Set up a duplicate call to hydrate()
Comment 2 Paul Derscheid 2025-06-16 15:33:52 UTC
Created attachment 183288 [details] [review]
Bug 40150: Prevent uncaught error on multiple attempts to 'define' on 'CustomElementsRegistry' in islands.ts

If the name is already in use, we just return.

It might make sense to have a console.debug() or similar to notify
developers that this is happening. Open for other opinions.

To test:
1) Apply the first patch which adds an additional hydrate call in main-container.inc
2) Go to acqui/acqui-home.pl
3) Verify that you see this error in the dev tool's console tab:
Uncaught (in promise) NotSupportedError: Failed to execute 'define' on 'CustomElementRegistry': the name "acquisitions-menu" has already been used with this registry
4) Apply the second patch
5) Run `yarn js:build` in ktd (assuming ktd).
6) Reload the page without cached assets (Ctrl/Cmd + Shift + R) and verify that the uncaught error is gone.
7) Sign off.

There's an additional problem here I (and probably you) noticed. But that's unrelated to this bug.
Comment 3 David Nind 2025-06-16 22:01:06 UTC
Created attachment 183292 [details] [review]
Bug 40150: Set up a duplicate call to hydrate()

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2025-06-16 22:01:09 UTC
Created attachment 183293 [details] [review]
Bug 40150: Prevent uncaught error on multiple attempts to 'define' on 'CustomElementsRegistry' in islands.ts

If the name is already in use, we just return.

It might make sense to have a console.debug() or similar to notify
developers that this is happening. Open for other opinions.

To test:
1) Apply the first patch which adds an additional hydrate call in main-container.inc
2) Go to acqui/acqui-home.pl
3) Verify that you see this error in the dev tool's console tab:
Uncaught (in promise) NotSupportedError: Failed to execute 'define' on 'CustomElementRegistry': the name "acquisitions-menu" has already been used with this registry
4) Apply the second patch
5) Run `yarn js:build` in ktd (assuming ktd).
6) Reload the page without cached assets (Ctrl/Cmd + Shift + R) and verify that the uncaught error is gone.
7) Sign off.

There's an additional problem here I (and probably you) noticed. But that's unrelated to this bug.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2025-06-16 22:06:49 UTC
Added assignee.

For some reason, the name for the [DO NOT PUSH] patch was changed. I'll obsolete my signed off patch.

> There's an additional problem here I (and probably you) noticed. But that's unrelated to this bug.

[Violation] 'setTimeout' handler took 62ms?

Otherwise, no idea. 8-)