outline / rich-markdown-editor

The open source React and Prosemirror based markdown editor that powers Outline. Want to try it out? Create an account:
https://www.getoutline.com
BSD 3-Clause "New" or "Revised" License
2.87k stars 588 forks source link

Impossible to install via `npm` due to conflicting and duplicating dependencies. #512

Open arslancharyev31 opened 3 years ago

arslancharyev31 commented 3 years ago

rich-markdown-editor version: 11.17.0

rich-markdown-editor defines an explicit dependency on prosemirror-tables, which at the moment resolves to 1.1.1. However, rich-markdown-editor also defines an explicit dependency on prosemirror-utils , which at the moment resolves to 0.9.6. The problem is that prosemirror-utils@0.9.6 also defines an explicit dependency on prosemirror-tables, which resolves to 0.9.5. The following tree illustrates the resulting dependency graph:

my-react-app@0.1.0 /path/to/project
└─┬ rich-markdown-editor@11.17.0
  ├── prosemirror-tables@1.1.1
  └─┬ prosemirror-utils@0.9.6
    └── prosemirror-tables@0.9.5

This results in RangeError: Duplicate use of selection JSON ID cell exception being thrown since prosemirror-tables tries to define some unique element twice (correct me if I'm wrong here).

And no, this is not an issue with "unique package situation", as was suggested in https://github.com/outline/rich-markdown-editor/issues/412#issuecomment-803216815. On yarn this issue can be somewhat mitigated through "selective dependency resolutions". (Update: I just checked, and apparently in yarn all packages resolve to prosemirror-tables@1.1.1, hence manual resolution is not required here). But npm provides no such mechanism at the moment (its counterpart, overrides, is an approved but not yet implemented rfc). Of course one could manually edit the package-lock.json to resolve it, but this is a highly suboptimal solution for self-evident reasons. Therefore, this issue has to be fixed either in upstream or in this repository.

The good news is that this issue is fixed in prosemirror-utils@1.0.0-0, which no longer defines an explicit dependency on prosemirror-tables, which should, in theory, fix this issue. The bad news is that 1.0.0-0 is tagged as release candidate, which may or may not be acceptable to maintainers of rich-markdown-editor.

Finally, having said all that, would you consider upgrading the problematic prosemirror-utils package to v1, so that npm users can use rich-markdown-editor?

tommoor commented 3 years ago

Unfortunately it's not possible to upgrade to 1.0.0-0 as it removes a number of methods for dealing with tables that this project uses. I guess that's why the dependency was removed

arslancharyev31 commented 3 years ago

Unfortunately it's not possible to upgrade to 1.0.0-0 as it removes a number of methods for dealing with tables that this project uses. I guess that's why the dependency was removed

That is indeed unfortunate. I suppose at the very least it should be stated in the README that npm is not supported for reasons that were described in my original post. At the very least it would definitely save time for future developers who are trying to install it according to the instructions but are met with really bizarre errors.

guiguan commented 3 years ago

Got the same problem when trying to load this into Deno via esm.sh

colmanhumphrey commented 3 years ago

We have the same problem, temporarily solved by using yarn for the deploy, ignoring the package-lock file.

alexef commented 2 years ago

535 is a duplicate of this.

arslancharyev31 commented 2 years ago

Unfortunately it's not possible to upgrade to 1.0.0-0 as it removes a number of methods for dealing with tables that this project uses. I guess that's why the dependency was removed

By the way, this issue is a textbook example of what eventually happens when a project relies on transitive dependencies.

DhawalAgrawal commented 2 years ago

Any possible solution to this bug?

tommoor commented 2 years ago

Someone would need to figure out what changed in prosemirror-transform that causes the weird behavior in newer versions.

Once that's addressed the dep locking is no longer needed

adrm commented 2 years ago

I think having a dependency on yarn over npm is not a good situation. If that is accepted as a minor evil, that should be clearly stated in the installation instructions on the readme, at least.

For anyone trying to use this from npm, the only fix right now is to add this to your package.json:

"overrides": {
  "prosemirror-tables": "^1.1.1"
}

Then, delete your package-lock.json file and node_modules folder, and run npm install.

ariane-codes commented 2 years ago

I think having a dependency on yarn over npm is not a good situation. If that is accepted as a minor evil, that should be clearly stated in the installation instructions on the readme, at least.

For anyone trying to use this from npm, the only fix right now is to add this to your package.json:

"overrides": {
  "prosemirror-tables": "^1.1.1"
}

Then, delete your package-lock.json file and node_modules folder, and run npm install.

Thanks! This worked for me. :)