Open daKmoR opened 6 years ago
I think this is outside the scope of all current bundlers / module installers. For instance, npm
and yarn
(or webpack
) have no such protection mechanisms do they?
JavaScript registers web components using an imperative API (customElements.define
) and the "name" field in package.json
has no bearing on what element[s] that code might register.
copied over from issue on package-name-maps:
@arcanis Actually, it's not a requirement of pnm - but first recap the main goal of pnm as far as I understood.
pnm should enable us to use bare modules within the browser without a build step
e.g. you can upload your full directory/source code as is and put it on a plain web/file server and it should work.
Keeping that in mind it means that we may have a lot of files on our server but we still want to ship as little a possible to our users/browser.
Naturally, that means we don't want to ship code doing the same thing multiple times. So using like 4 versions of for example camelcase
in our application may be a bad idea regarding the file size - but it should still work. That is the reason why pnm supports multiple versions of a package. I would still find a warning, in that case, would be appropriate [1].
For custom elements it's just not possibele to use different versions of the same element. It's a requirement of CustomElementRegistry. The relevant part is
If this CustomElementRegistry contains an entry with name name, then throw a "NotSupportedError" DOMException.
In good old english this means: "Each custom elements tag name needs to be unique within a browsers Window object".
Actually from a browsers point of view it's obvious there can only be one <video>
tag.
Also a nice comment about it is https://github.com/domenic/package-name-maps/issues/5#issuecomment-374175653.
PS: there have been some workaround like creating unique custom element names by adding random strings but they definitely violate the idea to only ship as little as possible.
[1]: This could be more than just a warning. You could even give some info on how to pin to certain versions that should be used. e.g. something like this,
The package "camelcase" is used with the following versions 1.2.2, 1.3.0, 2.0.0, 2.0.9. You can run "pnm set-resolution camelcase 1.3.3 2.1.0" to only use the latest releases of each major version.
PS: Have a safe flight š
@wmhilton: neither yarn nor npm have a way to override or limit the amount of versions that you use. In webpack you can do use resolutions to archive it and/or use tools like duplicate-package-checker-webpack-plugin to get a warning.
So, in short, you can solve it with build tools but not with packager tools. There are some movement in this direction but no working implementation yet[1] and changes to npm and yarn always take quite some time and usually have a huge impact (as so many people use it)
So having a specialized generator for package name maps that support a unique flag wound a preferred solution I would say. And it would definitely be awesome.
JavaScript registers web components using an imperative API (customElements.define) and the "name" field in package.json has no bearing on what element[s] that code might register.
That is true but we are not interested in the name (as on package can have multiple custom elements) we just want to make sure that certain packages are only ONCE in the package name map.
e.g.
import 'fireCollection/my-foo.js'; // customElements.define('my-foo', ...)
import 'fireCollection/my-bar.js'; // customElements.define('my-bar', ...)
is totally fine as long as fireCollection
is only ONCE available. As es modules will never load a same file twice... e.g. if you later do import 'fireCollection/my-foo.js';
nothing will happen.
However if those are different versions then they resists in different locations so it will be loaded again... and boom DOMException
[1]: if you exclude yarn --flat
as it will mingle up your devDependencies which you want deep (only your "browser" dependencies you want flat)
I am curius where you wana go with it e.g. what is planned? for example did you see the possible overhaul of the package-name-maps (maybe even with rename)? domenic/package-name-maps#53
So this project started as a proof-of-concept. I wanted to show that it was possible (and easy) to generate any type of data structure simply by extracting the data from the PnP API. Since the pnm proposal is something I kept an eye on, it was a good candidate š
Overall, I think it's important to stay close to the layout provided by the package manager. As you mentioned, the goal of pnm (if I understood correctly) is to be able to use the dependency as-is, by serving the installed app. To make it truly isomorphic, it has to use the same dependency tree than the one used by Node.
Naturally, that means we don't want to ship code doing the same thing multiple times. So using like 4 versions of for example camelcase in our application may be a bad idea regarding the file size
While I understand the size concern, I'm concerned about the burden of manually solving conflicts. That could in my mind be a big barrier to the pnm adoption. We would have to check in real case scenarios, but I suspect a lot of dependency ranges would conflict, requiring users to manually solve the conflicts (which is quite hard since they usually have no idea which one to use) š¤
There are some movement in this direction but no working implementation yet
I think I quite like what @ljharb suggested - to enforce the "flatness" by using peer dependencies. There are some pitfalls that have to be considered, though:
In its current state, it will break the "yarn add foo
and it works" workflow, since the user would also have to manually install the peer dependencies for foo
(it's already the case, but peer dependencies are relatively rare so that's fine). Maybe we could add a flag to tell that a peer dependency should be auto-added, but it seems a bit too specific to me.
The current PnP spec guarantees that a package with peer dependencies will be instanciated once for each time it is found in the tree. It's not a problem if it is a peer-dependency itself (because then it would only appear once in the dependency tree), but that makes it quite easy to get weird errors that might be hard to debug. Some tooling would be welcomed to ensure that we can catch those errors at install-time.
It only works if the ecosystem uses this pattern (meaning a lot of teaching, without guarantee that it will propagate enough), and it's a technical solution, not a semantical one (meaning that we won't be able to swap the implementation for a better one later on).
PS: Have a safe flight š
Thanks @daKmoR! š
I may have not explained well enough what I would expect š
I definitely don't want want to have a global flag so that users will need to choose for every package. But for packages that are "naturally" unique like web components it would help to catch the error while installing and not while opening the browser. Sending them both to the browser or even have a mapping for it is not usefull at all as you just can't load it.
I would like to quote @zkat here
The Node.js world, by contrast, has been much more aggressive about small-modules, and thus a potential increase in version conflicts, because the Node algorithm has always protected them from that.
for webcomponents in the browser directly there is NO protection we can offer - it's really awesome that you don't need to care about it in node (or even react?)... but if you use webcomponents the browser (rightfully so) forces you.
I'm sorry for beeing so persitent but it seem I may be a dreamer that still believes in a world where you don't need any build tools while developing. (I do not consider yarn install a build step)
Let my try slightly different. Let's extend my demo repository with another dependency
// package.json
"dependencies": {
"test-webcomponent-feature-a": "^0.0.2",
"test-webcomponent-feature-b": "^0.0.3",
"webpack": "^4.0.0"
},
something like this should give me the question "Could not find a sadisfying version for 'test-webcomponent ..." but ONLY because I have configured "test-webcomponent" to be unique. webpack will be installed nested if needed and nothing will change for it.
possible configuration for unique packages could be
// package.json
{
"name": "test-webcomponent",
"version": "2.0.0",
"devDependencies": {
"http-server": "^0.11.1"
},
"build-pnm": {
"unique": true
}
}
to summarize:
TLDR; @daKmoR I completely disagree with your reasoning but agree that the requested feature is a good one.
The disagreeing part:
I still think you're conflating WebComponents with modules.
Let's say I have a module that exports a webcomponent, MyFoo
.
You could have multiple versions of that module in the graph. You just need to make sure that only one copy of the module actually registers MyFoo
. The best modules export pure functions, and don't run any code when they are imported, and certainly don't register globals. You don't need to bring WebComponents into the conversation to see this - all we need to do is think back to ye old jQuery days. We don't want multiple versions of jQuery attempting to attach themselves to the window
object.
It's pretty straightforward to say, "well, if you're publishing WebComponents that auto-register on import, you're doing it wrong" and toss the problem to userspace.
BUT.
That's not very helpful. š¤·āāļø
The agreeing part:
@daKmoR brought up the duplicate-package-checker-webpack-plugin
. I use that plugin extensively in my own work to prevent bundle bloat. And I do think that it would be awesome to have a way to force certain packages to use certain versions. That would let application developers solve/workaround the jQuery / WebComponent problem, regardless of whether module authors wrote misbehaving modules that register globals. Webpack in particular has a lot of flexibility to hack "workarounds" around existing packages that don't play well with the web. So tools that generate package name maps for the web should also (ideally) provide a similar level of hackability.
However, it seems very likely that the root of the issue is not in build-pnm
at all, but ought to be addressed in pnm
itself. If build-pnm
just uses the pnm
API, then adding custom module resolution aliasing and rewriting like we're describing would be a kind of tacky addon to this tool, imho. Since pnm
uses code to define it's module resolution, I feel like this could be addressed in a future upgrade to the pnm
library.
You just need to make sure that only one copy of the module actually registers MyFoo. The best modules export pure functions, and don't run any code when they are imported, and certainly don't register globals.
Yes and no. Imagine the following: you write a component Foo. Foo@1
accepts a prop named cb
, and Foo@2
renames it to callback
. Your application depends on Bar
(which depends on Foo@1
) and Baz
(which depends on Foo@2
).
Even if Foo
doesn't register itself automatically (let's say it exposes a setup
function that, when called, calls window.customElements.define('Foo', ...)
), you still can't call the setup
from both Foo@1
and Foo@2
- meaning that one of Bar
and Baz
will break, depending on which version of Foo
got its setup
function called. So it's not only a problem of impure packages, but rather the sheer fact that some packages are designed to be singletons, for the better or the worse.
I think more generally the problem of singleton packages is something that should be solved by the package managers. It's far from being a Javascript-only problems: taking C and C++ as example, you can't link identical symbols together (unless static storage). This is the same here.
The main question I'm still thinking about is whether it should come from the user or from the package. Should the user specify that package Foo should be singleton'd? Or should the package Foo says that it must be singleton'd (similar in some way to the flat: true
settings we currently have on Yarn)?
but rather the sheer fact that some packages are designed to be singletons, for the better or the worse.
yes that is the main point š¤
very simple example
// v1.0.0/foo.js
window.fooFlag = 'BAR';
// v2.0.0/foo.js
window.fooFlag = 'BAZ';
// app.js
import 'v1.0.0/foo.js';
if (window.fooFLAG === 'BAR') { ... } // e.g. depending on v1
so if ANY package you use is now loading the v2 version... this will just break... problem is it will silently break and stuff like that is usually tough to find and fix.
I think more generally the problem of singleton packages is something that should be solved by the package managers
I totally agree that it should be solved on the npm or yarn side... but to be honest I have given up on that hope š (at least on the hope that it happens any time soon). So I hoped for the package name maps - but maybe you are right and it's the wrong place to do it by default... writing something on top of that to give warnings or errors for double versions like the plugin for webpack should be easily double.
The main question I'm still thinking about is whether it should come from the user or from the package.
I would go with that the package author sets it... he knows if the package code needs to be unique or not. Checking for every dependency that I have if it should be unique or not would be tough - especially for transitive dependencies.
so if I do npm i foo
it could be that it installs 3 packages "deep" and 2 packages as singleton... and foo itself could allow for "deep" installs or not (e.g is itself a singleton or not)
First thanks for starting something like this š
I am curius where you wana go with it e.g. what is planned? for example did you see the possible overhaul of the package-name-maps (maybe even with rename)? https://github.com/domenic/package-name-maps/issues/53
I would like to use something like this to generate maps that can be used with web components. However they need to be unique.
Given the following dependency tree
We should get an Error/Question while creating package name map.
Possible example flow:
I made an example Repository https://github.com/daKmoR/build-pnm-webcomponent-issue
and published all needed packages to test it.
I'm curious on what you think? if I can help out in any way just let me know.