v2.0.0-beta.205 breaks strict package managers (monorepos)

myovchev commented 1 year ago

What’s the bug you are facing?

Since v2.0.0-beta.205 (introduced day before this issue is posted) prosemirror-* packages are refactored as peerDepdendencies in all tiptap packages, while before that they are dependecies. This IMHO is a very bad move and relies on the default npm behavior to resolve peerDependencies, but it breaks the work of good folks like pnpm, rush, etc that are trying to bring package management to a better place. All those are not resolving auto-magically the peerDeps but are expecting the developer to reference them explicitly.

To understand why this is bad, I was forced to add to my rush monorepo the following dependencies on an application level in order to resolve failing builds:

    "prosemirror-commands": "^1.3.1",
    "prosemirror-keymap": "^1.2.0",
    "prosemirror-schema-list": "^1.2.2",
    "prosemirror-dropcursor": "1.5.0",
    "prosemirror-gapcursor": "^1.3.1",
    "prosemirror-history": "^1.3.0"

The actual dependency depth to @tiptap/* and related with the failing build packages:

app/
   apostrophe/
       @tiptap/vue-2
       @tiptap/extension-highlight"
       @tiptap/extension-link"
       @tiptap/extension-placeholder"
       @tiptap/extension-text-align"
       @tiptap/extension-text-style"
       @tiptap/extension-underline"

The ApostropheCMS is internally building (Admin UI) @tiptap/vue-2 and extensions in their own build pipeline (webpack) totally independent from the application - you can think of the package apostrophe as a library (sort of). This change forces the ApostropheCMS team to introduce basically all prosemirror-* as direct dependencies in order to not break applications using different than npm (stricter) package manager. This being their responsibility (or any other library implementing tiptap) makes no sense to me.

Which browser was this experienced in? Are any special extensions installed?

Any

How can we reproduce the bug on our side?

Install an apostrophe application in a Rush monorepo. Generally the problem can be observed if @tiptap/* packages are dependencies of root application dependency and the packages are installed via pnpm or rush (I'm sure it breaks a lot more monorepo tools as well).

Can you provide a CodeSandbox?

No response

What did you expect to happen?

Convert the unnecessary peerDependencies back to dependencies.

Anything to add? (optional)

No response

Did you update your dependencies?

[X] Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

[x] Yes, I’m a sponsor. 💖

jacekkarczmarczyk commented 1 year ago

duplicate of #3492

myovchev commented 1 year ago

I'm sorry about that, but I wasn't able to find relevant by title issue (the referenced issue is about a concrete implementation).

bdbch commented 1 year ago

Hey, just a heads up on this.

We moved to peerDependencies because there were a lot of version issues in the past specially with the prosemirror-state package which would lead to duplicate plugin key errors and JSON id errors on runtime.

We tried to resolve this a few weeks ago by fixing prosemirror versions inside the packages as dependencies but the issue still popped up when users were also adding their own prosemirror versions into their projects.

We'll discuss in the team tomorrow what we're going to do with those issues. One thing would be to go back to prosemirror-deps in each extension where they're needed and keep the peerDependency for @tiptap/core only.

I'll update this issue when we talked about this. Thanks for your patience

myovchev commented 1 year ago

@bdbch Thanks for the quick response and the additional info. We were able to temporary resolve the situation and our builds are working again.

One suggestion though - even if @tiptap/core only introduces peer dependencies (thus the same applies to @tiptap/vue-2) the app developers will end up with the same problem. I fully understand that you (and we all) fight a bad legacy design decisions of npm. A good compromise would be an official notifications for a breaking change from your side after your final decision about the peer deps and before the corresponding release implementing it. This would ensure all vendors implementing the library can react and adapt dependencies based on your recommendation beforehand. Breaking change on a beta channel is not exactly semver compliant but in this edge case and with the appropriate communication is an acceptable solution.

bdbch commented 1 year ago

We had a quick talk this morning but were a bit time-capped. While we really don't like the experience right now (in terms installing all prosemirror-* deps on your own when using pnpm / yarn) we believe it's kind of the right move.

We'll have another talk later where we'll see we can improve this. What we can make sure for now is that we'll add better documentation for a change like this, for example inside our release note + add new installation instructions to our docs if the peerDependencies stay.

In the end we'll need to make a decision between:

A. Making sure the installation is easy and uncomplicated but could introduce version clashes with prosemirror as prosemirror is very sensitive with multiple prosemirror versions being loaded which would need some kind of info for Tiptap devs on how to resolve those issues somehow

or

B. Making sure Tiptap works on it's own and prosemirror needs to be provided by the devs themselves to make sure only one source of truth exists for prosemirror which would lead to a worse setup when it comes to installing and setting up Tiptap which we could and should cushion via better documentation and upgrade guides / notes.

I'll come back to the issue later with more info

coader commented 1 year ago

so now we need to add those peer dependence manually?

tmikaeld commented 1 year ago

so now we need to add those peer dependence manually?

Not if you install it with npm, since it takes care of peer dependencies

coader commented 1 year ago

I downgrade to 205 or 2.04 it also throw error, because the starter-kit use the latest core:

coader commented 1 year ago

I have to add those in packages and it can run

myovchev commented 1 year ago

The peerDependencies should be auto-resolved when using npm v7+ and not providing the --legacy-peer-deps flag. The pnpm package manager has a dedicated option for auto resolving peer deps. I'm not aware what's the situation with yarn. RushJS configured with npm v7+ fails to resolve the newly introduced prosemirror-* peer deps but this seems to be a rush related problem.

@bdbch node v14 comes with npm v6 by default. This results in peer deps not being resolved. The easy solution is to explicitly update npm to e.g. v8. This fact might generate a number of failed builds and increased support effort so clearly mentioning it in the docs sounds a wise move. Apostrophe CMS will add notes on that matter in the upcoming release.

bdbch commented 1 year ago

We added information about the peerDeps in our installation guide in the docs: https://tiptap.dev/installation#1-install-the-dependencies

But I'm also working on a better solution right now that should help out with those long install lines for Tiptap since our team also doesn't really like the current approach.

What we're working on is a meta package like @tiptap/prosemirror that you can install to automatically import all required prosemirror dependencies. in combination, you could also access prosemirror exports via import { EditorState } from '@tiptap/prosemirror/state.

I'm not 100% done as I have to do a bit of an overhaul on our build scripts to separate that build step from the other packages.

bdbch commented 1 year ago

so now we need to add those peer dependence manually?

Yes for now. As @myovchev explained this is how for example yarn does package resolving for peer deps.

softbeehive commented 1 year ago

First of all, I appreciate your hard work on the great open-source editor 🙏

Though I have to say in this situation DX is far from pleasant because folks on classic yarn 1.x including myself can't even downgrade back to pre 2.0.0-beta.205

I was trying to understand why lock file contains references to 205 when I pin 202. Installing dependencies of tiptap explicitly is forcing people to maintain them.

This is sub-optimal because:

I don't know what version of prose-mirror packages each new release of tiptap is compatible with.
Eventually after upgrading packages I'm gonna end-up with spaghetti mix of versions and run into a conflict.
I don't use npm because it lacks features like resolutions

markhughes commented 1 year ago

Could this be caused by the tiptap prosemirror tables extension?

my yarn.lock has it as this:

"@tiptap/prosemirror-tables@^1.1.3":
  version "1.1.3"
  resolved "https://registry.yarnpkg.com/@tiptap/prosemirror-tables/-/prosemirror-tables-1.1.3.tgz#cd812487fc103f921299a0cf899485f147b418b2"
  integrity sha512-Pp7Iyup+kxniDGEvFHX+BRw3et9vys83NVqk6B+PbVAztq64QyX/mY+vzcpFmOsF9/th+Po981igbGUG7C/8hw==
  dependencies:
    prosemirror-keymap "^1.1.2"
    prosemirror-model "^1.8.1"
    prosemirror-state "^1.3.1"
    prosemirror-transform "^1.2.1"
    prosemirror-view "^1.13.3"

Here is my packages:

    "@tiptap/core": "2.0.0-beta.207",
    "@tiptap/extension-blockquote": "2.0.0-beta.207",
    "@tiptap/extension-bold": "2.0.0-beta.207",
    "@tiptap/extension-bullet-list": "2.0.0-beta.207",
    "@tiptap/extension-character-count": "2.0.0-beta.207",
    "@tiptap/extension-code": "2.0.0-beta.207",
    "@tiptap/extension-code-block": "2.0.0-beta.207",
    "@tiptap/extension-color": "2.0.0-beta.207",
    "@tiptap/extension-document": "2.0.0-beta.207",
    "@tiptap/extension-gapcursor": "2.0.0-beta.207",
    "@tiptap/extension-hard-break": "2.0.0-beta.207",
    "@tiptap/extension-heading": "2.0.0-beta.207",
    "@tiptap/extension-history": "^2.0.0-beta.207",
    "@tiptap/extension-horizontal-rule": "2.0.0-beta.207",
    "@tiptap/extension-image": "2.0.0-beta.207",
    "@tiptap/extension-italic": "2.0.0-beta.207",
    "@tiptap/extension-link": "2.0.0-beta.207",
    "@tiptap/extension-list-item": "2.0.0-beta.207",
    "@tiptap/extension-ordered-list": "2.0.0-beta.207",
    "@tiptap/extension-paragraph": "2.0.0-beta.207",
    "@tiptap/extension-strike": "2.0.0-beta.207",
    "@tiptap/extension-table": "2.0.0-beta.207",
    "@tiptap/extension-table-cell": "2.0.0-beta.207",
    "@tiptap/extension-table-header": "2.0.0-beta.207",
    "@tiptap/extension-table-row": "2.0.0-beta.207",
    "@tiptap/extension-text": "2.0.0-beta.207",
    "@tiptap/extension-text-align": "2.0.0-beta.207",
    "@tiptap/extension-text-style": "2.0.0-beta.207",
    "@tiptap/extension-underline": "2.0.0-beta.207",
    "@tiptap/extension-youtube": "2.0.0-beta.207",
    "@tiptap/prosemirror-tables": "^1.1.3",
    "@tiptap/react": "2.0.0-beta.207",
    "prosemirror-commands": "^1.5.0",
    "prosemirror-dropcursor": "^1.6.1",
    "prosemirror-gapcursor": "^1.3.1",
    "prosemirror-history": "^1.3.0",
    "prosemirror-keymap": "^1.2.0",
    "prosemirror-model": "^1.18.3",
    "prosemirror-schema-list": "^1.2.2",
    "prosemirror-state": "^1.4.2",
    "prosemirror-transform": "^1.7.0",
    "prosemirror-view": "^1.29.1",

However, interestingly enough, I no longer get issues about keys when I drop the @tiptap/extension-gapcursor@2.0.0-beta.207 plugin

Even with my resolutions set up and only one version being resolved, I still get complaints about the keys:

RangeError Adding different instances of a keyed plugin (plugin$)

prosemirror-commands@1.5.0, prosemirror-commands@^1.5.0:
  version "1.5.0"
  resolved "https://registry.yarnpkg.com/prosemirror-commands/-/prosemirror-commands-1.5.0.tgz#d10efece1647c1d984fef6f65d52fdc77785560b"
  integrity sha512-zL0Fxbj3fh71GPNHn5YdYgYGX2aU2XLecZYk2ekEF0oOD259HcXtM+96VjPVi5o3h4sGUdDfEEhGiREXW6U+4A==
  dependencies:
    prosemirror-model "^1.0.0"
    prosemirror-state "^1.0.0"
    prosemirror-transform "^1.0.0"

prosemirror-dropcursor@1.6.1, prosemirror-dropcursor@^1.6.1:
  version "1.6.1"
  resolved "https://registry.yarnpkg.com/prosemirror-dropcursor/-/prosemirror-dropcursor-1.6.1.tgz#31f696172105f232bd17543ccf305e0f33e59d1d"
  integrity sha512-LtyqQpkIknaT7NnZl3vDr3TpkNcG4ABvGRXx37XJ8tJNUGtcrZBh40A0344rDwlRTfUEmynQS/grUsoSWz+HgA==
  dependencies:
    prosemirror-state "^1.0.0"
    prosemirror-transform "^1.1.0"
    prosemirror-view "^1.1.0"

prosemirror-gapcursor@1.3.1, prosemirror-gapcursor@^1.3.1:
  version "1.3.1"
  resolved "https://registry.yarnpkg.com/prosemirror-gapcursor/-/prosemirror-gapcursor-1.3.1.tgz#8cfd874592e4504d63720e14ed680c7866e64554"
  integrity sha512-GKTeE7ZoMsx5uVfc51/ouwMFPq0o8YrZ7Hx4jTF4EeGbXxBveUV8CGv46mSHuBBeXGmvu50guoV2kSnOeZZnUA==
  dependencies:
    prosemirror-keymap "^1.0.0"
    prosemirror-model "^1.0.0"
    prosemirror-state "^1.0.0"
    prosemirror-view "^1.0.0"

prosemirror-history@1.3.0, prosemirror-history@^1.3.0:
  version "1.3.0"
  resolved "https://registry.yarnpkg.com/prosemirror-history/-/prosemirror-history-1.3.0.tgz#bf5a1ff7759aca759ddf0c722c2fa5b14fb0ddc1"
  integrity sha512-qo/9Wn4B/Bq89/YD+eNWFbAytu6dmIM85EhID+fz9Jcl9+DfGEo8TTSrRhP15+fFEoaPqpHSxlvSzSEbmlxlUA==
  dependencies:
    prosemirror-state "^1.2.2"
    prosemirror-transform "^1.0.0"
    rope-sequence "^1.3.0"

prosemirror-keymap@1.2.0, prosemirror-keymap@^1.0.0, prosemirror-keymap@^1.1.2, prosemirror-keymap@^1.2.0:
  version "1.2.0"
  resolved "https://registry.yarnpkg.com/prosemirror-keymap/-/prosemirror-keymap-1.2.0.tgz#d5cc9da9b712020690a994b50b92a0e448a60bf5"
  integrity sha512-TdSfu+YyLDd54ufN/ZeD1VtBRYpgZnTPnnbY+4R08DDgs84KrIPEPbJL8t1Lm2dkljFx6xeBE26YWH3aIzkPKg==
  dependencies:
    prosemirror-state "^1.0.0"
    w3c-keyname "^2.2.0"

prosemirror-model@1.18.3, prosemirror-model@^1.0.0, prosemirror-model@^1.16.0, prosemirror-model@^1.18.3, prosemirror-model@^1.8.1:
  version "1.18.3"
  resolved "https://registry.yarnpkg.com/prosemirror-model/-/prosemirror-model-1.18.3.tgz#d1026a78cff928fd600e90d87cf7d162e0a4e3fd"
  integrity sha512-yUVejauEY3F1r7PDy4UJKEGeIU+KFc71JQl5sNvG66CLVdKXRjhWpBW6KMeduGsmGOsw85f6EGrs6QxIKOVILA==
  dependencies:
    orderedmap "^2.0.0"

prosemirror-schema-list@1.2.2, prosemirror-schema-list@^1.2.2:
  version "1.2.2"
  resolved "https://registry.yarnpkg.com/prosemirror-schema-list/-/prosemirror-schema-list-1.2.2.tgz#bafda37b72367d39accdcaf6ddf8fb654a16e8e5"
  integrity sha512-rd0pqSDp86p0MUMKG903g3I9VmElFkQpkZ2iOd3EOVg1vo5Cst51rAsoE+5IPy0LPXq64eGcCYlW1+JPNxOj2w==
  dependencies:
    prosemirror-model "^1.0.0"
    prosemirror-state "^1.0.0"
    prosemirror-transform "^1.0.0"

prosemirror-state@1.4.2, prosemirror-state@^1.0.0, prosemirror-state@^1.2.2, prosemirror-state@^1.3.1, prosemirror-state@^1.4.1, prosemirror-state@^1.4.2:
  version "1.4.2"
  resolved "https://registry.yarnpkg.com/prosemirror-state/-/prosemirror-state-1.4.2.tgz#f93bd8a33a4454efab917ba9b738259d828db7e5"
  integrity sha512-puuzLD2mz/oTdfgd8msFbe0A42j5eNudKAAPDB0+QJRw8cO1ygjLmhLrg9RvDpf87Dkd6D4t93qdef00KKNacQ==
  dependencies:
    prosemirror-model "^1.0.0"
    prosemirror-transform "^1.0.0"
    prosemirror-view "^1.27.0"

prosemirror-transform@1.7.0, prosemirror-transform@^1.0.0, prosemirror-transform@^1.1.0, prosemirror-transform@^1.2.1, prosemirror-transform@^1.7.0:
  version "1.7.0"
  resolved "https://registry.yarnpkg.com/prosemirror-transform/-/prosemirror-transform-1.7.0.tgz#a8a0768f3ee6418d26ebef435beda9d43c65e472"
  integrity sha512-O4T697Cqilw06Zvc3Wm+e237R6eZtJL/xGMliCi+Uo8VL6qHk6afz1qq0zNjT3eZMuYwnP8ZS0+YxX/tfcE9TQ==
  dependencies:
    prosemirror-model "^1.0.0"

prosemirror-view@1.29.1, prosemirror-view@^1.0.0, prosemirror-view@^1.1.0, prosemirror-view@^1.13.3, prosemirror-view@^1.27.0, prosemirror-view@^1.28.2, prosemirror-view@^1.29.1:
  version "1.29.1"
  resolved "https://registry.yarnpkg.com/prosemirror-view/-/prosemirror-view-1.29.1.tgz#9a4938d1a863ca76e23c6573d30e3ece2b17d9a0"
  integrity sha512-OhujVZSDsh0l0PyHNdfaBj6DBkbhYaCfbaxmTeFrMKd/eWS+G6IC+OAbmR9IsLC8Se1HSbphMaXnsXjupHL3UQ==
  dependencies:
    prosemirror-model "^1.16.0"
    prosemirror-state "^1.0.0"
    prosemirror-transform "^1.1.0"

myovchev commented 1 year ago

We added information about the peerDeps in our installation guide in the docs: https://tiptap.dev/installation#1-install-the-dependencies

But I'm also working on a better solution right now that should help out with those long install lines for Tiptap since our team also doesn't really like the current approach.

What we're working on is a meta package like @tiptap/prosemirror that you can install to automatically import all required prosemirror dependencies. in combination, you could also access prosemirror exports via import { EditorState } from '@tiptap/prosemirror/state.

I'm not 100% done as I have to do a bit of an overhaul on our build scripts to separate that build step from the other packages.

This indeed sounds like a much more elegant solution. :100:

SalahAdDin commented 1 year ago

We are getting this problem only with the Garpcursor extension.

SamuelMS commented 1 year ago

@bdbch Love the idea of a @tiptap/prosemirror package – we'll likely hold off on upgrading until that gets set up since our team uses yarn extensively. Should we just keep an eye on #3556 for updates?

bdbch commented 1 year ago

Hey @SamuelMS yes I'll keep you updated as soon as we have news on this. I'll focus on this issue for this and next week so we should have something up soon.

bdbch commented 1 year ago

I just merged a PR for this which brings a few bigger breaking changes. Keep a look out for the release + new changelog.

bdbch commented 1 year ago

@myovchev Our new version is released. Make sure to update to 2.0.0-beta.211

tmikaeld commented 1 year ago

@bdbch Does it work with Yarn now?

bdbch commented 1 year ago

It should work if you install @tiptap/pm

SalahAdDin commented 1 year ago

@myovchev Our new version is released. Make sure to update to 2.0.0-beta.211

You can read more here: https://tiptap.dev/blog/new-pm-package-and-upgrade-guide-for-beta-210

I will check it soon!

ueberdosis / tiptap