material-components / material-web

Material Design Web Components
https://material-web.dev
Apache License 2.0
9.06k stars 857 forks source link

Use bare specifier import for dependent components #5107

Open augustjk opened 9 months ago

augustjk commented 9 months ago

What is affected?

Tooling

Description

As reported by a user on discord https://discord.com/channels/1012791295170859069/1076197552904491108/1165169545816846346

when components are loaded from CDNs like esm.run or esm.sh, they end up being bundled such that using multiple components can cause duplicate custom element registration errors.

CDN bundlers currently have no knowledge of all the entrypoints to the package to be able to deduplicate internal self imports. Using bare specifiers and package exports would help with this.

Reproduction

https://lit.dev/playground/#gist=73234f9745308e38a224d3eb92ca71ab

doing

import "https://esm.sh/@material/web/button/elevated-button.js";
import "https://esm.sh/@material/web/switch/switch.js";

produces error

Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "md-focus-ring" has already been used with this registry

Workaround

Workaround is to use the all.js entrypoint to register everything at once, or don't use a CDN and use a build system to actually bundle the code being used yourself.

Using bare specifier imports internally would allow better deduplication control by these bundlers. e.g. https://github.com/material-components/material-web/blob/33e4293eca4f7056ed106f7247f12424dbc82290/button/internal/button.ts#L7-L8

could turn into

import '@material/web/focus/md-focus-ring.js';
import '@material/web/ripple/ripple.js';

Unfortunately, I think package exports is required for self reference in Node https://nodejs.org/api/packages.html#self-referencing-a-package-using-its-name. So following should be added to package.json

{
  "exports": {
    "./button/*.js": "./button/*.js",
    "./checkbox/*.js": "./checkbox/*.js",
    ... etc
  }
}

This can be considered breaking unless absolutely every file that was accessible before, including internal files, is listed there.

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

1.0.1

Browser/OS/Node environment

n/a

kkachniarz220 commented 6 months ago

This never worked, can custom elements creation error be catched internally? Or smth like this

if(!customElements.get('foo')){
  customElements.define('foo', Foo);
}