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

Pinned Watchdog version leads to problems when updating #376

Closed
Witoso opened this issue Jun 21, 2023 · 2 comments · Fixed by #378
Closed

Pinned Watchdog version leads to problems when updating #376

Witoso opened this issue Jun 21, 2023 · 2 comments · Fixed by #378
Assignees
Labels
squad:devops Issue to be handled by the Devops team. type:bug
Milestone

Comments

@Witoso
Copy link
Member

Witoso commented Jun 21, 2023

In the published package on the npm, we pin a specific version of Watchdog which leads to problems when someone is using a different version of the editor or wants to update.

package.json on npm:
image

React on the other hand specifies Watchdog as a peer dependency:
image.

Not sure what's the best approach to solve this but it's a major issue for some integrators.

@Reinmar
Copy link
Member

Reinmar commented Jun 22, 2023

We also planned to expose Watchdog on the Editor class to completely avoid this additional dependency. 

Some internal notes where we discussed this:

  • Update (13.04.2023):
    • 😂 And the above didn’t work: Expose Watchdog and Context on Editor class and update integrations.  ckeditor5#13852
    • Two options for now:
      • Add ckeditor5-watchdog to the “Framework” layer and make the base Editor expose these
        • Simpler option
        • Risk: circular dependency on types between the editor and watchdog interfaces
        • This time let’s also update the integrations to verify that it all works together
      • Merge the code from ckeditor5-watchdog to ckeditor5-core, but keep ckeditor5-watchdog as a re-export for backwards compat
        • It would make core the dep of watchdog pkg (it has no deps now)

@pomek pomek added the squad:devops Issue to be handled by the Devops team. label Jun 28, 2023
@pomek
Copy link
Member

pomek commented Jun 29, 2023

Scope

  • We agreed to define core, engine, utils, and watchdog packages as peer dependencies.
    • They all are needed when an integrator installs the <CKEditor> component and tries to build a custom app.
    • Example: tsc needs to resolve import type { GetEventInfo } from '@ckeditor/ckeditor5-utils';, so the utils package must be installed too.
    • We require at least version 37 or higher (>=37.0.0). 37 is the first number that offers typings.
  • It's a breaking change.
  • Update the Angular guide. We should mention that peer dependencies should be installed in the same version like the editor build that integrator wants to use.

@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Jul 3, 2023
@psmyrek psmyrek self-assigned this Jul 7, 2023
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Jul 7, 2023
pomek added a commit that referenced this issue Jul 10, 2023
Other: The following packages are now marked as peer dependencies [`@ckeditor/ckeditor5-core`](https://www.npmjs.com/package/@ckeditor/ckeditor5-core), [`@ckeditor/ckeditor5-engine`](https://www.npmjs.com/package/@ckeditor/ckeditor5-engine), [`@ckeditor/ckeditor5-utils`](https://www.npmjs.com/package/@ckeditor/ckeditor5-utils) and [`@ckeditor/ckeditor5-watchdog`](https://www.npmjs.com/package/@ckeditor/ckeditor5-watchdog) to avoid issues when upgrading the CKEditor 5 version within the angular application. Closes #376.

MAJOR BREAKING CHANGE: The[ `@ckeditor/ckeditor5-angular`](https://www.npmjs.com/package/@ckeditor/ckeditor5-angular) package requires the following peer dependencies:

* [`@ckeditor/ckeditor5-core`](https://www.npmjs.com/package/@ckeditor/ckeditor5-core),
* [`@ckeditor/ckeditor5-engine`](https://www.npmjs.com/package/@ckeditor/ckeditor5-engine),
* [`@ckeditor/ckeditor5-utils`](https://www.npmjs.com/package/@ckeditor/ckeditor5-utils),
* [`@ckeditor/ckeditor5-watchdog`](https://www.npmjs.com/package/@ckeditor/ckeditor5-watchdog).

Make sure to install them in the same version as the editor build you want to use in your integration.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jul 10, 2023
pomek added a commit to ckeditor/ckeditor5 that referenced this issue Jul 10, 2023
Docs: Added documentation in Angular guide that `ckeditor5-angular` package requires peer dependencies. See ckeditor/ckeditor5-angular#376.
@CKEditorBot CKEditorBot added this to the iteration 65 milestone Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:devops Issue to be handled by the Devops team. type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants