esm.run: importing from the same library breaks singletons

[claviska]

Describe the bug

Using /+esm to import more than one ES module from the same library results in shared modules being bundled, effectively breaking singleton patterns.

The behavior is demonstrated in this pen:

https://codepen.io/claviska/pen/vYJyVYb?editors=1000

Affected jsDelivr links

This import loads an icon component that imports a base path utility from dist/utilities/base-path.js:

https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@2.0.0-beta.57/dist/components/icon/icon.js/+esm

This import loads the base path utility directly:

https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@2.0.0-beta.57/dist/utilities/base-path.js/+esm

In the first URL, you'll see the base path module has been bundled into the icon module. (It's hard to see because the code is mangled, but search for document.getElementsByTagName("script") which is unique to the base path module.)

In the example, the user imports a method called setBasePath() that updates a singleton value. Because the module is bundled with the icon, the singleton is broken resulting in icons having the wrong base path.

Additional context

Contrast the broken example above to unpkg's ?module which works as expected:

https://codepen.io/claviska/pen/eYEBPYR?editors=1000

In this case, the base path module is not bundled with the icon module.

Using singletons in JS modules is a common pattern that lets us scope important values to modules rather than relying on globals. If you need additional information or have any questions, I'm happy to help however I can!

[MartinKolarik]

Hi @claviska, thanks for the detailed report.

This is an issue we're aware of but I don't see any great way to prevent it. The pattern that you use to make this work relies on an internal per-file state, which works when either all of the files are left in the graph without bundling (the unpkg's approach) or when everything is bundled into one file - i.e. the user imports just the one main file, which then exports everything he needs ("kitchen sink" build).

If your project has many components and you want to support loading those individually, you can still have one file for each of them bundled separately, but they can't internally import a module that keeps the state like in this case. You'd have to have e.g. a global "loader" component that manages this information.

As per the first paragraph in this comment, another way to "fix" this would be introducing a way to disable the bundling and just rewrite all paths as unpkg does. But if we actually did that, you'd find that then we also make all those extra requests unpkg does (because many small files instead of a few larger ones) and that the load performance suffers, so would you actually recommend your users to do that?

We'd like this feature to work in as many cases as possible and not require the package authors to write the code in a specific way, but again, I don't see a great solution in this case.

[claviska]

Thanks for the fast, detailed reply! I understand that this isn't very straightforward. 😕

If your project has many components and you want to support loading those individually, you can still have one file for each of them bundled separately, but they can't internally import a module that keeps the state like in this case. You'd have to have e.g. a global "loader" component that manages this information.

Alas, its very difficult to enforce this pattern, especially over time and with contributors who may not be aware. It's also not obvious that things will break until after it hits the CDN. Sadly, I'm not sure it's possible to lint for this pattern. 🤔

But if we actually did that, you'd find that then we also make all those extra requests unpkg does (because many small files instead of a few larger ones) and that the load performance suffers, so would you actually recommend your users to do that?

I'd be OK with that. This is what Shoelace was doing up until a few releases ago (previously deps were bundled as part of the build, but the goal is to use /+esm to ship deps with bare specifiers instead). I never heard a single complaint from end users regarding CDN delays.

However, Shoelace may be unique in that it only has a handful of dependencies and uses esbuild to efficiently chunk shared code. I can see how projects with a lot of dependencies might suffer, though.

I'm not sure what the best way forward is. I appreciate the concern for performance, but a way to opt out of bundling (either in package.json or as part of the URL) would be a welcome feature.

[MartinKolarik]

a way to opt out of bundling (either in package.json or as part of the URL) would be a welcome feature.

I can see some value in such option but we'll have to think it over since once we add it, there's no going back.

[justinfagnani]

If your project has many components and you want to support loading those individually, you can still have one file for each of them bundled separately, but they can't internally import a module that keeps the state like in this case.

This completely breaks standard module semantics. One huge reason to even make a separate module is so you can have common state like caches. Packages I maintain, like lit-html and LitElement, use things like per-module caches, class statics (that are effectively per-module). Not sharing the caches will cause a lot of duplicated work that'll slow down a page.

Unbundled like unpkg would be a better approach. While it can result in duplicate packages due to the greedy/local version resolution, it'll only duplicate as much there's version differences.

A much better approach would be to allow users to create a "project" with a package.json that specifies a bunch of package/versions to use so that resolution can be done up front and each request with the same project ID in the URL would use the same resolution results. That would have less duplication than even plain npm.

[MartinKolarik]

This completely breaks standard module semantics. One huge reason to even make a separate module is so you can have common state like caches. Packages I maintain, like lit-html and LitElement, use things like per-module caches, class statics (that are effectively per-module). Not sharing the caches will cause a lot of duplicated work that'll slow down a page.

Just to clarify what we mean here. Are you saying you have many "modules" within one npm package, which can be imported independently by the end users but share some state via shared files in that package? Generally, the internal state is not a problem, of course, it's only a problem when it's split across multiple files which are loaded independently like in this issue and if it's all within a single npm package (we don't bundle external deps).

[justinfagnani]

Are you saying you have many "modules" within one npm package, which can be imported independently by the end users but share some state via shared files in that package?

Yes, absolutely. This is what modules enable, and we've been doing this with lit-html for 4+ years now. It'll be even more common with Node's ESM support and package exports. Package exports let you very explicitly define multiple module entrypoints into a package.

[MartinKolarik]

Yes, absolutely. This is what modules enable, and we've been doing this with lit-html for 4+ years now.

Ok, well, it's definitely a valid pattern, just a very problematic one for bundling done at the modulepackage level.

Yes, absolutely. This is what modules enable, and we've been doing this with lit-html for 4+ years now. It'll be even more common with Node's ESM support and package exports. Package exports let you very explicitly define multiple module entrypoints into a package.

Package exports might actually improve the situation because they at least provide some information in return - if a package has 5 entry points, we can analyze which files are shared by them and exclude those from bundling. But if any file is a possible entry point (like now), we simply can't bundle anything without having the risk of this problem.

[justinfagnani]

If bundling w/o package exports implies duplicating shared modules, then it should almost definitely be turned off, IMO.

To put the scope of the problem in perspective, lit has 32 entrypoints, most uses will import more than one, and many simple use cases could import from 4+ modules resulting in many copies of the core Lit modules. Other packages are moving towards multiple entrypoints too, so this isn't only a Lit issue.