Importing web components in plugins throws exceptions

[fatih-erikli]

I received the following exception when I tried to import modal-dialog in the plugin I currently work on.

DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "modal-dialog" has already been used with this registry.

Doing a control like this

if (typeof customElements.get("modal-dialog") === "undefined") {
  customElements.define("modal-dialog", ModalDialog);
}

Fixes the problem when registering the modal dialog web component. However I am not sure if this is a correct way to handle that. I open this issue for brainstorming.

[justvanrossum]

That is most certainly not the correct solution to the problem.

So apparently the modal-dialog module gets imported more than once: we need to find out why, and how to prevent it.

Does it relate to the import map? If so, we could try to normalize all imports to use the import map spelling. That is a very wild guess.

[fatih-erikli]

Modal-dialog gets imported in editor.js, and in the plugin file.

[justvanrossum]

Modal-dialog gets imported in editor.js, and in the plugin file.

Normally, when a module is imported in multiple places, the runtime recognizes that it's the same module, and loads it only once. This happens all over the place in Fontra, and works correctly.

But it goes wrong in our plugin situation, so the module is actually loaded twice, hence the error. So we need to find out:

• Why is the module as imported from the plugin not recognized as the same module
• How can we fix it so it is seen as the same module.

My initial suspicion is that the import map may play a role in this.

[justvanrossum]

My initial suspicion is that the import map may play a role in this.

To test this, you can try to use the import map spelling in the core code, and see if the error disappears.

[fatih-erikli]

Looks like import maps is the problem. It works without any problem when loading the modules with a / prefix.

[justvanrossum]

I mean the opposite: can you try to use the import map spelling in the core code?

Like this:

• "fontra/web-components/modal-dialog.js"

instead of

• "/web-components/modal-dialog.js"

[fatih-erikli]

It didn't help, I face with same problem. I tried to replace all the imports (including relative imports) with fontra/ prefix. I receive the element has been already defined error. This happens only when the module is loaded with the import map.

It works without any problem if we skip using import maps.

[fatih-erikli]

If a module is imported twice, the component is trying to be registered twice (or the number of how many times it is imported in all the core code).

[justvanrossum]

Hm, I'm trying to experiment with importmap imports and I'm also getting some unexpected results.

However, I am now wondering, perhaps it is the cache busting trick that causes the trouble: the plug-in source will not be preprocessed with the version token.

Can you try with a disabled cache busting mechanism?

diff --git a/src/fontra/core/server.py b/src/fontra/core/server.py
index 77db33f0..c68c01d6 100644
--- a/src/fontra/core/server.py
+++ b/src/fontra/core/server.py
@@ -287,6 +287,7 @@ class FontraServer:
         return web.Response(body=html, content_type="text/html")
 
     def _addVersionTokenToReferences(self, data: bytes, contentType: str) -> bytes:
+        return data
         if self.versionToken is None:
             return data
         jsAllowedFileExtensions = ["css", "js", "svg"]

[justvanrossum]

One thing I discovered (but is perhaps obvious?): the import map only works for import statements in .js files, not in html "imports", like these:

    <script type="module" src="/web-components/ui-list.js"></script>

[justvanrossum]

If I arbitrarily replace some (but not all) "/web-components/modal-dialog.js" imports to "fontra/web-components/modal-dialog.js" imports, everything keeps working as expected.

The more I think about it the more confident I am the problem you're seeing is caused by the cache busting mechanism. I will have to try to think how we could solve that, because we have no control about how the plugin assets are served.

[justvanrossum]

We could try to add a redirect to the server code: if it receives a request for some-module.js (without version token), it could redirect to some-module.<version-token>.js. That might be worth a try.

[justvanrossum]

FWIW:

everything keeps working as expected

Only with the "no version token" hack in place. This is an unrelated problem: the version token code is too picky about paths: it requires the import path to start with . or /, so fontra/... is not recognized.

[justvanrossum]

We could try to add a redirect to the server code

I tried that but it doesn't work: the browser does not look at the redirected path, so it will still not recognize the module as being the same.

[fatih-erikli]

I'm thinking out loud. If we download the plugin to the local plugins folder in plugin manager, the jsdelivr service no longer be needed. Then the import maps no longer be needed.

[justvanrossum]

I'm thinking out loud. If we download the plugin to the local plugins folder in plugin manager, the jsdelivr service no longer be needed. Then the import maps no longer be needed.

This would require downloading all assoiciated assets, too. It has security issues, too. I thought of it, too, but I think it is a big can of worms.

Here is another possible solution:

• adjust all (absolute) imports to use the fontra/ importmap prefix
• the server can dynamically replace the actual path with one that has a version token:
  • /core/foo.js imported as fontra/core/foo.js becomes /jsXXXXX/core/foo.js
  • the server recognizes these requests, and serves the files from their real location

While this may be a big change because it changes many imports, the mechanism (at least for .js files, it doesn't work for other assets, but that's less of a problem) is cleaner than what we have now.

Let's think about this, but in the meantime please continue your task, with the version token mechanism disabled in your local code base.

[fatih-erikli]

The task #1072 is done if we ignore the import maps problem. It works when you replace /fontra with / in start.js of the plugin. https://github.com/blackfoundrycom/fontra-reference-font.

Maybe we create a refactoring tasks for all the imports to be replaced with "/fontra" in the core code?

[justvanrossum]

Thanks!

The task https://github.com/googlefonts/fontra/issues/1072 is done if we ignore the import maps problem

Do you mean the version token problem?

It works for me (directly from the plugin repo) if I disable the version token as described here: https://github.com/googlefonts/fontra/issues/1084#issuecomment-1932768026

Maybe we create a refactoring tasks for all the imports to be replaced with "/fontra" in the core code?

Yes, that should be a separate task, I will open one, once I am confident that it will be the best solution. (At the moment, I'm a bit worried about possible merge conflicts with open PRs.)