Make ckeditor5 a peer dependency of all other packages

[Reinmar]

The next iteration (iteration19) will contain some breaking changes (e.g. #1038, https://github.com/ckeditor/ckeditor5-core/issues/64), so it's a good moment to squeeze some more breaking/infrastructural things too.

There's one idea about making ckeditor5 a peer dependency of other source packages (and a real dependency of builds). I explained it in #667 (comment).

I'm not sure whether we'll be able to also work on reducing dependency on other pkgs, but those changes will be a progressive enhancement, so we can do them later on. Adding ckeditor5 everywhere, though, is something which will affect other people, so we should do it now.

[Reinmar]

I'm leaving this as a "discussion" because we haven't made a final decision yet. I want to track it, though.

[Reinmar]

BTW, it came to my mind what we could put in ckeditor5. See #1005 (comment).

Since adding ckeditor5 as a peer dep of all other packages is meant to define the version of the environment in which this feature was meant to work, checking if ckeditor5 got installed once (by validating global CKEDITOR_VERSION variable) will bring additional level of security.

[Reinmar]

@pomek, could you test whether out release tools will properly bump version of ckeditor5 if it's a peer dependency of other packages? I guess not, because I don't think we're touching the peerDependencies key at all.

I've been also thinking a bit about releasing all this and it seems that... we'll need to release all packages at the same time – ckeditor5-* and ckeditor5. Otherwise, the version of ckeditor5 bumped in ckeditor5-* pkgs will make it impossible to install these packages. So, you could also check in our complete release process what blocks us from making the process continuous from the npm run release:dependencies to npm run release.

[pomek]

@pomek, could you test whether out release tools will properly bump version of ckeditor5 if it's a peer dependency of other packages? I guess not, because I don't think we're touching the peerDependencies key at all.

You're right. This tool does not touch peerDependencies key.

[pomek]

The release tool now supports updating the peerDependencies (ckeditor/ckeditor5-dev#420).

I also introduced dry run mode which allows testing everything without pushing/publishing anything. We talked about it last time. It's very helpful.

I was able to generate changelogs for all packages (I used the 12.0.0 version). Then I run the release script and everything worked fine. The version of ckeditor5 as a peer dependency was updated.

I also didn't have any troubles with executing the whole release process.

I'm preparing pull requests now.

[Reinmar]

Some findings...

• ckeditor5 became a dependency of ckeditor5-utils because it needs to access ckeditor5/package.json. ckeditor5-utils is a dependency of every package (through ckeditor5-core or ckeditor5-engine or ckeditor5-ui). Which means that ckeditor5 will always be installed.

• In the future, once adding plugins to builds will be possible, we would like to verify that a plugin is compatible with the build by defining peeeDependencies to ckeditor5 on both of them (the build and the plugin). However:

  • We can't add ckeditor5 as a peer dependency of ckeditor5-build-* because it'd require always calling npm i ckeditor5 ckeditor5-build-foo every time you want to install a build. This is a no-go.
  • Adding ckeditor5@^1.0.0 as a dependency of a build and ckeditor5: ^2.0.0 as a peer dependency of some plugin will log a warning (because ckeditor5@^2.0.0 wasn't installed). This warning will not be logged if the versions where matching (because ckeditor5-build-* provided ckeditor5).
  • Now... normally, when installing a plugin in which you already have e.g. ckeditor5-editor-classic you won't have to install ckeditor5 manually because it was installed by ckeditor5-utils (we're talking here about a case of building CKEditor 5 from source). Which means that if you'll see this warning after installing a plugin next to a build (assuming that it has an incompatible version of ckeditor5 in its dependencies) you may realise that they are incompatible with each other.
  • OTOH, If you'll install ckeditor5@^2.0.0 manually to satisfy that warning (which I'm afraid many developers will do seeing that warning) nothing will be logged. In that case, you'll have two versions of ckeditor5 installed at once. So it does not save us from installing two versions of ckeditor5 at all and thus mean that nothing gets verified for 100%.
  • However, all the above happens only if ckeditor5-plugin does not have ckeditor5-core and ckeditor5-engine in its dependencies. If it has, they will cause installing ckeditor5-utils, which will cause installing ckeditor5 even if its version doesn't match the one already installed... Hence, nothing gets verified. This might potentially change in the future, once plugins will need to depend on ckeditor5-core or ckeditor5-engine less often. However, some utils will never be exposed globally, hence, many plugins will still import stuff from ckeditor5-utils or ckeditor5-engine... Which means that ckeditor5 will again be provided automatically so in case of conflicts it will be duplicated. LOL...

• Warning logged by npm is quite easy to be missed:

  npm WARN @reinmar/test-peer-dep-a@1.0.0 requires a peer of @reinmar/test-peer-dep@^1.0.0 but none was installed.
  

  Usually, there are many other warnings too when you run npm i on a bigger project.

• ckeditor5-build-* are versioned in the same way as ckeditor5 (versions are equal) which means that adding ckeditor5 there won't make a big difference in clarifying what's your CKEditor 5 version.

• Currently, you're able to track with which editor version a certain plugin (e.g. a 3rd part one) works by checking which ckeditor5-engine and ckeditor5-core it requires. We bump up these versions on every release currently. This, however, means that if we'll make it possible to write most of the plugins without depending on ckeditor5-engine or ckeditor5-core, checking the compatible version of CKEditor 5 will be hard. That's a moment when adding peerDependencies will really help, although, I have a feeling that it's more of a documentation than verification. It, in a standardised way, allows a consumer to check whether a certain package can be used with their CKEditor 5 installation.

Initial conclusions

• npm won't verify anything. It won't prevent any situations where incompatible versions of packages are installed. It may help to notice them, but nothing's sure.
• As long as we import ckeditor5-engine and ckeditor5-core in plugins, we don't really need ckeditor5 in their peerDependencies.
• However, it's a safe and future-compatible addition which may help people to better understand the status of some plugins (e.g. 3rd party ones) so we could still consider adding it now to all plugins.

Other considerations

I've checked that webpack uses peerDependencies for its plugins. Babel doesn't do it (at least for the latest stable release).

In webpack, the version of webpack in its plugins peerDependencies is set to e.g.:

• ^4.0.0
• ^4.0.0 || ^3.0.0 || ^2.0.0

It's always ^N.0.0.

This is important because we were thinking with @pomek whether we should always bump ckeditor5's version once a package is released. A wider range (^N.0.0) will be easier to fulfill by a certain setup of packages. However, let's consider this scenario:

1. Step 1. Packages as follow:
   • ckeditor5@v1.0.0
   • ckeditor5-engine@v1.0.0
   • ckeditor5-paragraph@v1.0.0 depends on ckeditor5-engine
2. Step 2. Engine has a breaking change. But it's not a breaking change in some of its utils:
   • ckeditor5@v1.0.1 (yep, it might have been just a bug fix considering the entire project)
   • ckeditor5-engine@v2.0.0
   • ckeditor5-paragraph@v1.0.1 depends on ckeditor5-engine

Now, in step2, if we'll go webpack's way, ckeditor5-paragraph will still have ckeditor5@v1.0.0 as its peerDependency. However, it may be completely incompatible with ckeditor5-engine@v1.0.0 which is part of a range ckeditor5@^1.0.0. Now, imagine that ckeditor5-paragraph doesn't have ckeditor5-engine in its dependencies (which we plan to make possible in the future). Yes... it's going to break badly.

Which means that we have three options:

• Always make a major release of ckeditor5 when one of the packages had a major release. Which means that 50%+ releases will be major release which will be confusing for "editor consumers" (where the "editor" changes far less frequently).
• Always bump ckeditor5 in peerDependencies and hardcode it to a specific version A.B.C. That will lead to releasing all packages when a single package is changed because a single package release causes also releasing ckeditor5 which means... that all other packages have to have empty releases too just to update ckeditor5 version in their peerDependencies.
  • It will ensure that every month every plugin has a release. This will show that they are alive and it's kinda correct to say that "we bumped ckeditor5 in peerDependencies because this plugin was tested with this specific editor release". We can also recommend the community to use wider range in their 3rd party plugins to avoid problems with installing them.
  • The downside is that it's ugly. Changelogs will be full of empty releases and we'll have to have special sections for empty releases in the ckeditor5's changelog so people are able to check what really changed...
• Don't remove ckeditor5-engine and ckeditor5-core from plugin dependencies. Or, at least leave the dependency on ckeditor5-core. Then, we could still treat ckeditor5-core as a kind of a version indicator of the engine/core APIs (so, the APIs used by plugins). We could also have a rule that a major bump in ckeditor5-engine will be a major bump in ckeditor5-core since most of the engine API is exposed in editor.* props. API consumers will be able to use as wide ranges for ckeditor5-core as they tested are valid for them.

For now, I'm inclined towards the last option. However, in general, all this means that it doesn't make sense to make any changes now. As long as plugins depend on the ckeditor5-engine and ckeditor5-core we're "fine". There are other problems (like packages getting duplicated) which we have to work on first and with time we'll also better understand how 3rd party plugins should be published.

Final conclusions

1. Document that plugincollection-plugin-name-conflict is thrown when adding plugins to a build or when a plugin is installed twice. I finally understood why this error is quite high in our stats. (@Reinmar)
2. Make sure that version of ckeditor5 is bumped in ckeditor5-utils's dependencies. (@pomek)
3. Work on a "package duplication" check for CKEditorWebpackPlugin() because it's the ugliest possible issue now. It could also warn when it detects that an existing build is being compiled together with some source packages. (@ma2ciek)
4. Once the above is done we'll be in situation when end-consumers are quite safe and aware of problems if problems appear. But we'll still have to make it easier to write simple plugins.
5. Expose remaining ckeditor5-engine's APIs (ranges, positions, converters) and review plugins to remove ckeditor5-engine from as many of them as possible.
6. For prototyping plugins without importing anything we still need two more things:
   • editor.commands.add( 'name', commandProtype ) so you don't have to import the Command class. However, I would keep the current commands implemented in the way they are (because it makes it easy to test them and reuse them). This is just to enable prototyping things.
   • Expose basic UI components in EditorUI (button, basic dropdown, split button). This list could be compatible with a declarative API for defining toolbar when dropdowns and split buttons.
7. After we'll do all that, the only real issue will be that adding existing plugins (implemented with imports) to builds will still not be possible. But let's do it step by step and return to this problem when we'll solve the rest.

[pomek]

Make sure that version of ckeditor5 is bumped in ckeditor5-utils's dependencies. (@pomek)

Works fine.

[pjasiun]

editor.commands.add( 'name', commandProtype ) so you don't have to import the Command class.

It's a very nice idea. In fact, many commands define only execute methods. Some others overwrite also refresh method, but it could be handled by events. So commandProtype could be in fact the execute function.

Some plugins also have a very strong dependency between command and editing part of the plugin. For such plugins, it might be a better idea to keep the whole logic in a single file.

[Reinmar]

I improved the error message in ckeditor/ckeditor5-dev@263f37b. The next step:

Work on a "package duplication" check for CKEditorWebpackPlugin() because it's the ugliest possible issue now.

[wwalc]

Regarding a possibly different approach regarding peer dependencies.

I made a private "contrib" emoji plugin and was wondering on how to resolve the trouble of outdated dependencies. I don't want to update my plugin whenever the new version of CKEditor 5 is out. At the same time leaving outdated dependencies may lead to problems.

I made my plugin based on the link plugin, so initially I put into the dependencies the same packages (core, engine, ui). Then I realised that these packages are actually so common and should be present in any editor build, that it probably makes no point in listing them as dependencies.

Maybe such dependencies should go into peerDependencies?

[Reinmar]

I've been so focused on making ckeditor5 a peer dependecy of other packges that I missed that it can be much simpler, like you proposed. The engine, core and the UI libs are part of the "framework" and need to be installed by the integrator (just like you install webpack). Not by plugins. I really like this!

[Reinmar]

BTW, you could test it by updating your plugin :)

[wwalc]

I published an updated version of a package, but due to defining there the latest peerDependencies, the only setup that I may check at this stage is a project in which outdated core/engine/ui dependencies are used together with the emoji plugin which defines the latest stable versions as peerDependencies.

The result is quite okay when testing it with ckeditor5-build-classic: dependencies are not installed twice (at all). I don't quite get it though why npm does not complain at all about too old versions of installed dependencies, see below:

peerDependencies of the emoji plugin:

  "peerDependencies": {
    "@ckeditor/ckeditor5-core": "^11.0.0", // <--- 11.0.0 (!)
    "@ckeditor/ckeditor5-engine": "^10.2.0",
    "@ckeditor/ckeditor5-ui": "^11.0.0"
  },

dummy project without any dependencies

(a warning that dependencies are missing, cool)

dummy project with ckeditor5-core@10.0.0 (!) installed

(why no warning?)

In any case, even if npm (4.6.1) does not warn me about the version, it looks quite okay to me.

Oops, stupid me :D Actually npm still warns about the invalid version of core, but no longer complains about the engine. So everything is fine 👍

[wwalc]

One more comment: defining peerDependencies with caret ranges (^) will be still confusing for end users. Even if dependencies are not installed twice with different versions, the warning displayed to users may still cause them to install manually e.g. outdated versions of core/engine/ui.

Thus, I believe, 3rd party plugin maintainers should rather use >= in version fields:

  "peerDependencies": {
    "@ckeditor/ckeditor5-core": ">=10.0.0",
    "@ckeditor/ckeditor5-engine": ">=10.0.0",
    "@ckeditor/ckeditor5-ui": ">=10.0.0"
  },

At least this is what I will be doing from now. I know that this is not safe as one never knows if the plugin will work with all the future versions of the editor, but overall this should result in less issues.

[ma2ciek]

Thus, I believe, 3rd party plugin maintainers should rather use >= in version fields:

I don't like this idea. You don't know what will happen in the next major version.

Pinging the author to update package's deps and check the package is a better idea than installing broken package and pinging the author to fix it.

[Reinmar]

Pinging the author to update package's deps and check the package is a better idea than installing broken package and pinging the author to fix it.

And waiting a week? Month? A year? :P Realistically speaking, 90%+ packages will never be updated after having their initial release.

[wwalc]

Pinging the author to update package's deps and check the package is a better idea than installing broken package and pinging the author to fix it.

We already had a chance to test it once back in CKEditor 4 when we introduced an incompatible change which required updates in skins in the addons repository. The reply from maintainers was very poor even if we told them precisely what has to be changed in their skins. So in reality this does not work simply.

[pjasiun]

I agree with @wwalc and @Reinmar, actually wanted to propose the same. We should recommend using >= in the plugins which will not be updated often. I believe, that in most cases, even backward incompatible changes in core, engine or UI libs should not break these plugins. However, it is allways up to the developer. If he prefer to keep it safe and update the plugin each time editor is released, he can use ^.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).