html-webpack-plugin should be a peer dependency, or not directly require()'d

[arcanis]

As it happens; webpack-subresource-integrity makes a require('html-webpack-plugin') call, but doesn't list it anywhere in its dependencies (not even as a peer dependency).

While it works on most installs at the moment, it's only by accident, and package managers offer no guarantee that this will hold true much longer; in fact, it breaks now with Plug'n'Play and likely with pnpm as well.

Adding webpack-subresource-integrity as a peer dependency would have the unfortunate side effect that users would get a warning, but that's a separate (non-blocking, I hope, since it's purely visual) problem that should be solved on the package managers side 🙂

[jscheid]

Thanks for your bug report. Could you provide a test case or sample repository demonstrating the issue?

[jscheid]

From the linked ticket I can get a vague idea of what the problem is.

However, we can't declare html-webpack-plugin a peer dependency because this plugin doesn't depend on it - it works fine without it.

In the other ticket you describe what we're doing a "hack". That's fair enough, do you have a suggestion as to what else we should be doing?

[arcanis]

Peer dependencies are not mandatory. If you list it in your peerDependencies and your users don't add to their apps you'll still get a working application in the end - it's just that your package won't be able to require it.

Yarn (and maybe npm, I'm not sure what they do in these circumstances) is a bit overly cautious by adding warnings when this happens, but these warning were there more as a workaround (to add "actionnable" warnings) than anything. Plug'n'Play is a much better way to report to the user when there's something invalid in the app (such as a peer dependency not available).

As a side note, I just noticed that webpack is also missing from your peer dependencies (it doesn't cause a problem in Angular's case because of a compatibility fallback we have, but could potentially cause issues as well down the road).

[arcanis]

If you want to repro it, you can check out https://github.com/arcanis/wsi-example

Make sure you have Yarn 1.12, then run yarn --pnp && yarn node index.js in the repository.

[jscheid]

Thanks for the repro.

Have another look, we do have webpack in our peer dependencies.

Before we go further, do I understand correctly that everything is working fine for everyone, except for users of a new yarn feature marked as experimental?

And do I understand correctly that this experimental yarn feature doesn't have a good story for handling optional dependencies?

[arcanis]

Have another look, we do have webpack in our peer dependencies.

Sorry, got confused by an error message I need to improve. Still valid for html-webpack-plugin 🙂

Before we go further, do I understand correctly that everything is working fine for everyone, except for users of a new yarn feature marked as experimental?

It can break for everyone even now. Any change in the hoisting (which both Yarn and npm are free to do) could break your plugin, because the package managers are simply not aware of the links between it and html-webpack-plugin.

So yep, it (maybe? pnpm likely doesn't support it either) works, but it relies on an undefined behavior, to speak in stricter terms. I'd expect it to break even now in some fringe cases.

And do I understand correctly that this experimental yarn feature doesn't have a good story for handling optional dependencies?

I'm not sure I follow. Peer dependencies are the way you should have optional dependencies, as explained.

[jscheid]

Ok, thanks for elaborating. I see your point about hoisting and undefined behaviour and I'll add the peer dependency.

Peer dependencies are the way you should have optional dependencies

Since you seem to be involved in yarn development, let me point out that this also feels like a hack... a dependency that causes a warning when missing is not truly an optional dependency.

You've said that yarn is "overly cautious" but isn't there more to it? In this example, webpack is a mandatory peer dependency without which the plugin doesn't work at all, while HWP isn't, so it seems there should be a way of describing that difference to the package manager. That way it could print a warning for the mandatory peer and silently ignore the other.

[arcanis]

I'll think more about it, but the main issue is to find an API that would work regardless of the package manager (from my experience, reaching a consensus is a bit hard).

Thinking about it, maybe a pseudo-protocol would be backward-compatible and provide this kind of annotation? Something like this:

{
  "peerDependencies": {
    "html-webpack-plugin": "optional:^x.y.z"
  }
}

Since the peerDependencies ranges are purely a hint, it just would be ignored by package managers that wouldn't recognize the annotation 🤔

[jscheid]

That might work. Another solution could be an additional key, `optionalPeerDependencies" that lists names of peers that are optional; that would also be backward compatible.

Plug'n'Play is a much better way to report to the user when there's something invalid in the app (such as a peer dependency not available).

Am I correct in assuming that PnP would just always pull in HWP because we emit a require call to load it -- and that way, ultimately make it a non-optional dependency anyway?

[arcanis]

No, PnP doesn't make static analysis on the code itself, only on the dependencies reported in the package.json files. Our goal isn't to automatically install things you require, but rather to leverage the Yarn cache to fulfill all projects on the machine at once (instead of copying them around).

So the error thrown is at runtime - we inject a hook in the environment that replaces part of the Node resolution (the one that locates the "right" node_modules directory) by a resolution based on static resolution tables generated by yarn install. When this resolver sees a require that's not part of the tables (because Yarn wasn't aware of the connection), it throws. This ensures that packages can only access their direct dependencies rather than relying on the hoisting (hence solving cases of "works on my computer").

[jscheid]

Oh, I see... that's awesome.

[edmorley]

I'll think more about it, but the main issue is to find an API that would work regardless of the package manager

I've filed yarnpkg/yarn#6487 to continue exploring this, since I can think of several projects where the same issue will come up.