Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
myovchev opened this issue Dec 6, 2022 · 23 comments
Closed
2 tasks done

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

myovchev opened this issue Dec 6, 2022 · 23 comments
Labels
Type: Bug The issue or pullrequest is related to a bug

Comments

@myovchev
Copy link

myovchev commented Dec 6, 2022

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?

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

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@myovchev myovchev added the Type: Bug The issue or pullrequest is related to a bug label Dec 6, 2022
@jacekkarczmarczyk
Copy link

duplicate of #3492

@myovchev
Copy link
Author

myovchev commented Dec 6, 2022

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
Copy link
Contributor

bdbch commented Dec 6, 2022

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
Copy link
Author

myovchev commented Dec 7, 2022

@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
Copy link
Contributor

bdbch commented Dec 7, 2022

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
Copy link

coader commented Dec 9, 2022

so now we need to add those peer dependence manually?

image

@tmikaeld
Copy link

tmikaeld commented Dec 9, 2022

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
Copy link

coader commented Dec 9, 2022

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

@coader
Copy link

coader commented Dec 9, 2022

I have to add those in packages and it can run
image

@myovchev
Copy link
Author

myovchev commented Dec 9, 2022

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
Copy link
Contributor

bdbch commented Dec 10, 2022

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
Copy link
Contributor

bdbch commented Dec 10, 2022

so now we need to add those peer dependence manually?

image

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

@softbeehive
Copy link

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:

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

@markhughes
Copy link

markhughes commented Dec 11, 2022

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
Copy link
Author

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.
💯

@SalahAdDin
Copy link

We are getting this problem only with the Garpcursor extension.

@SamuelMS
Copy link

@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
Copy link
Contributor

bdbch commented Jan 19, 2023

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
Copy link
Contributor

bdbch commented Feb 2, 2023

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

@bdbch
Copy link
Contributor

bdbch commented Feb 2, 2023

@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

@bdbch bdbch closed this as completed Feb 2, 2023
@tmikaeld
Copy link

tmikaeld commented Feb 2, 2023

@bdbch Does it work with Yarn now?

@bdbch
Copy link
Contributor

bdbch commented Feb 2, 2023

It should work if you install @tiptap/pm

@SalahAdDin
Copy link

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

No branches or pull requests

9 participants