Use peerDependencies to avoid duplicate dependencies

[KamilaBorowska]

Currently none of CodeMirror projects declare peerDependencies. This tells the package manager that it can choose to have duplicate packages in node_modules, which is pretty bad when CodeMirror does depend on instanceof checks.

You may consider adding peerDependencies to avoid this issue. For example, in https://github.com/codemirror/view/blob/main/package.json the dependencies could be declared as follows:

  "dependencies": {
    "@codemirror/state": "^6.0.0",
    "style-mod": "^4.0.0",
    "w3c-keyname": "^2.2.4"
  },
  "peerDependencies": {
    "@codemirror/state": "^6.0.0",
    "style-mod": "^4.0.0"
  },

This tells the package manager that @codemirror/state and style-mod dependencies are part of a public API and as such it should always dedupe those.

This is not a theoretical issue, npm can choose to do so in certain situations, I noticed this particular issue with xfix/live@88d62fc. If package.json here gets replaced with the following:

{
  "name": "live",
  "private": true,
  "version": "0.0.0",
  "scripts": {
    "dev": "vite",
    "build": "vite build",
    "preview": "vite preview"
  },
  "devDependencies": {
    "vite": "^2.9.10"
  },
  "dependencies": {
    "codemirror": "^6.0.0",
    "@codemirror/lang-html": "^6.0.0"
  }
}

Then after running npm install (tested with npm 8.5.5 and 8.9.0), @codemirror/view will be in all those locations:

node_modules/@codemirror/lang-css/node_modules/@codemirror/view
node_modules/@codemirror/lang-html/node_modules/@codemirror/view
node_modules/@codemirror/lang-javascript/node_modules/@codemirror/view
node_modules/codemirror/node_modules/@codemirror/view

Which causes issues until npm dedupe gets executed.

[marijnh]

You may consider adding peerDependencies to avoid this issue.

I have considered that, but a part of the community is still using an npm version <6, which does not automatically install peer dependencies. This would make the library a huge pain to use for those people.

Also, in principle, npm will deduplicate properly on a fresh install now, if the version ranges are compatible. It's just when you upgrade or have otherwise complicated install procedures that it sometimes messes up and needlessly duplicates libraries. The situation seems to come up less frequently, once you no longer use 0.x versions, but yes, it unfortunately still comes up. You can usually work around it by removing your package lock and reinstalling.

[KamilaBorowska]

I have considered that, but a part of the community is still using an npm version <6, which does not automatically install peer dependencies. This would make the library a huge pain to use for those people.

I think that should work fine when a dependency is declared in both dependencies and peerDependencies. In npm 6, having a dependency listed in both will be intepreted as having a regular dependency, while since npm 7.5.5 it will be interpreted as a peer dependency (npm/rfcs#324).

[marijnh]

That's a very interesting trick. Do you know of any major packages that are using it, currently? Any potential pitfalls?

[KamilaBorowska]

That's a very interesting trick. Do you know of any major packages that are using it, currently?

All I was able to find (and I wrote a script that checked 1250 most popular dependencies) is https://github.com/tailwindlabs/tailwindcss, however because this was introduced before npm/rfcs#324 was accepted this is likely a mistake rather than intentional choice.

As for pitfalls, the biggest one I can think of that it's necessary to keep versions in sync - npm won't inform about mismatches and if mismatch occurs then different package managers will see different versions which could potentially cause issues.

[KamilaBorowska]

Albeit, thinking about it, this is not really necessary. Like, realistically CodeMirror is going to be used with extensions, and I think those extensions could easily declare a peerDependency for @codemirror/state and @codemirror/view because, well, the user is going to need a view in dependencies to use the editor (whatever it's provided by @codemirror/view or codemirror), no way around that. That could be good enough, the only issue is that it's not possible to tell about peer dependencies in @codemirror/view, but that may not be a big issue in practice.

[marijnh]

I've released @codemirror/autocomplete 6.0.2 with both peer- and regular dependencies. If that doesn't cause trouble, I'll do the same for other packages.

[KamilaBorowska]

I realized that this hack doesn't actually work :(. As a test, I started with this:

npm install --save @codemirror/autocomplete

Then I intentionally installed old version of @codemirror/state followed by a newer version:

npm install --save @codemirror/state@0.20.0
npm install --save @codemirror/state@6.0.0

This results in @codemirror/state being installed twice.

I think instead it may be reasonable to provide peerDependencies only for @codemirror/state and @codemirror/view as the user will need to have them installed anyway in order to use CodeMirror (like I did in codemirror/autocomplete#12). It's not ideal, but it's probably the best that can be done with npm 6 constraint.

[KamilaBorowska]

That said, for what it's worth, now that I checked, Yarn (both legacy and 3) does recognize having both dependencies and peerDependencies and reports conflicts, so maybe instead it may be worth having something like this for autocomplete:

  "dependencies": {
    "@codemirror/language": "^6.0.0",
    "@lezer/common": "^1.0.0"
  },
  "peerDependencies": {
    "@codemirror/language": "^6.0.0",
    "@codemirror/state": "^6.0.0",
    "@codemirror/view": "^6.0.0",
    "@lezer/common": "^1.0.0"
  },

@codemirror/state and @codemirror/view are installed as peer dependencies only to at least get some value in having peer dependencies when using npm, while @codemirror/language and @lezer/common are peer dependencies for Yarn.

[masad-frost]

I think a better approach is to not include dependencies at all. As someone who regularly uses codemirror in a large codebase, and regularly prototypes in small codebases, the pain that the dependency drift brings outweighs having to install the peer dependencies myself. This is especially true for things that can drift and duplicate without me noticing, @codemirror/state yells at me when there's multiple state packages, but I can end up with multiple versions of @lezer/common in my codebase and never notice. I have to perform lock file surgery often.

It makes sense to have dependencies for the codemirror package since it's a bundled package, but for others the user of the package should be able to control their package at the root level.

Here is a patch of what I'm suggesting codemirror/autocomplete@main...masad-frost:patch-1

[marijnh]

I think a better approach is to not include dependencies at all.

As I stated in a previous comment, I suspect that will be a huge pain for users without recent npm versions, who will have to manually make sure they include every @codemirror and @lezer package depended on by the stuff they use.

I feel the root problem here is npm inexplicably duplicating packages that don't need to be duplicated and not yet providing a reliable way to avoid this, and it unfortunately seems like there is not a lot package authors can do to avoid this problem.

[StoneCypher]

I have considered that, but a part of the community is still using an npm version <6, which does not automatically install peer dependencies. This would make the library a huge pain to use for those people.

another option is a postinstall

you can staple "pre" or "post" onto the name of any lifecycle script time and you get another script time

therefore "postinstall" is an event that runs after install, in which you can install peer deps, git submodules, python workspaces, or any other stuff like that

lots of tools check for a dep then if they see one don't check for the other dep types. this will go south if you start installing things like husky (admittedly i'm against husky, but still.)

this way you don't need any shenanigans against the concept of deps

[timcash]

I get the instance of error like this

Uncaught Error: Unrecognized extension value in extension set ([object Object]). This sometimes happens because multiple instances of @codemirror/state are loaded, breaking instanceof checks.
    at s (state.js:5:19110)
    at s (state.js:5:18701)
    at s (state.js:5:18701)
    at s (state.js:5:18701)
    at Qe (state.js:5:19303)
    at G.resolve (state.js:5:17832)
    at w.create (state.js:5:26718)
    at new C (view.js:3:51469)
    at codemirror.html:46:18

from this code

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <meta http-equiv="X-UA-Compatible" content="ie=edge" />
    <link rel="stylesheet" href="main.css" />
    <title>edit</title>
    <style>
      header {
        font-size: 2rem;
      }
      article {
        display: flex;
        flex-direction: column;
        align-items: left;
        padding: 30px;
        gap: 10px;
        height: 100vh;
      }

      .focus {
        color: var(--color-focus);
      }

      .aware {
        color: var(--color-aware);
      }

      li {
        color: gray;
      }
    </style>
  </head>
  <body>
    <article id="1">
      <header><span class="focus">Code</span> Mirror</header>
      <p>Edit Code with <span class="aware">Code Mirror</span></p>
    </article>
    <footer></footer>
    <script type="module">
      import { EditorView, basicSetup } from "https://esm.sh/codemirror";
      import { javascript } from "https://esm.sh/@codemirror/lang-javascript"


      let view = new EditorView({
        doc: 'console.log("Hello world")',
        extensions: [basicSetup, javascript()],
        parent: document.getElementById("1"),
      });
    </script>
  </body>
</html>

[steverep]

I think another argument for using peer dependencies is that it communicates the proper relationship to tools like dependabot, which otherwise will always treat each package independently no matter how big the version jump.

[acnebs]

I very much support this, as although it is true that deleting the lockfile and node_modules works, it also means having to wait 10 minutes to reinstall my entire node_modules for every debug iteration when I need to figure out which codemirror upgrade is breaking something (doing this at the moment).

[marijnh]

Is anyone still seeing this npm issue (pointless dependency duplication on upgrade) in recent versions of npm? I tried to isolate it but can't get it to happen anymore with 9.6 and up.

[zrosenbauer]

@marijnh I just ran into this issue today (using yarn) due to a mismatch between state and view (it requires older version of state).

I would recommend switching for these reasons:

• "As of npm v7, peerDependencies are installed by default"
• npm@6 is the lowest supported as far as I'm aware
• control > convience, I'd rather from a security, stability, etc. perspective control my deps

I'd also be happy to open a PR to do 2 things...

1. move to peer dep
2. add docs around the migration + update FAQs (if there are some... I'm not super familiar with the docs...)

[marijnh]

@marijnh I just ran into this issue today (using yarn)

Don't use yarn. If you use a recent version of npm, this problem shouldn't be common anymore (I'm not sure if it's entirely eliminated, but I can't get it to happen anymore).

[zrosenbauer]

@marijnh I just ran into this issue today (using yarn)

Don't use yarn. If you use a recent version of npm, this problem shouldn't be common anymore (I'm not sure if it's entirely eliminated, but I can't get it to happen anymore).

@marijnh LOL I don't think an npm package should drive which package manager folks are using, yarn is widely used (I think like ~40% of folks...) & would be the equivalent (honestly worse) of saying "we don't support firefox".

If I want to extend @codemirror/view I have to also include & install @codemirror/state. I don't believe thats a side-effect I'd expect, especially if you are separating the packages. If they are 100% dependent than I'd recommend including them in one package, otherwise why version separately OR breaking out Range utils or things like combineConfig into a utils lib?

Most of the usage (I spot checked) seemed to be mostly types no? If thats case this could be a devDependency (I think there was Range manipulation being done... soo maybe doesn't make sense).

Also I'll acknowledge I'm very new the CodeMirror/ProseMirror (❤️ the work done here).... but as outsider/new user I was caught off guard as most other core npm packages I use rely on peerDeps for things that aren't 100% tied together (could be my misunderstanding)

[marijnh]

LOL I don't think an npm package should drive which package manager folks are using

Sure. And I don't think companies should bring out broken tools that fragment the community duplicating standard tools, but we don't live in a perfect world, so here we are.

[zrosenbauer]

LOL I don't think an npm package should drive which package manager folks are using

Sure. And I don't think companies should bring out broken tools that fragment the community duplicating standard tools, but we don't live in a perfect world, so here we are.

I know messaging doens't get across emotion/sarcasms but from my perspective I don't think the snark is helpful to the conversation.

This is about how you design and ship dependencies. If you don't have a valid reason for not switching just say you don't want to do it as its too much work etc. (IMO as maintainer... that is very valid reason) and close the issue.

Hiding behind "I think yarn is dumb" isn't a valid reason + your discounting almost half of the potential users out there, many of which can't change due to their organization's choices (I choose yarn so I'm happy to work around on my end, which I did...).

Why build cool shit if you don't want ~half the people to use it?

[StoneCypher]

yarn is a relatively rare tool. no, it's not 40% of the community.

it's not every package maintainer's responsibility to modify their packages to cope with yarn's bugs.

provide a valid reproduction.

[marijnh]

A reproduction isn't needed. I know yarn and old npm versions do needless duplication when you upgrade a single package in an existing lock/dep tree. But peer dependencies have their own problem, and don't match the library's package structure well. So I consider the future to be modern npm with regular dependencies. If you must use yarn, don't use piecemeal upgrade, just blow away your package lock and reinstall everything.

I am closing this and asking everyone to stop responding in inflammatory ways.

[zrosenbauer]

I think closing is a good choice so folks know this won't change / happen + they can see the history of the conversation.

I'd like to sign off of this convo with @marijnh I appreciate the work you did & are continuing to do with CodeMirror and ProseMirror ❤️.

[acnebs]

This thread has actually made me finally switch from Yarn to pnpm 😆