search

ckeditor / ckeditor5-react

Official CKEditor 5 React component.
https://ckeditor.com/ckeditor-5
Other
426 stars 100 forks source link

Fixed resolving the peer dependencies #470

Closed DawidKossowski closed 7 months ago

DawidKossowski commented 7 months ago

Suggested merge commit message (convention)

Internal: Fixed resolving the peer dependencies.

Additional information

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

pomek commented 7 months ago

@DawidKossowski, could you give a short explanation of why this change is needed?

DawidKossowski commented 7 months ago

Sure. Now, every package used in the source code is included in the build, even if it's listed in peerDependencies. For instance, we use Watchdog version 40.1.0 in React integration, which is also listed in the peer dependencies. However, it gets bundled into the file: https://cdn.jsdelivr.net/npm/@ckeditor/ckeditor5-react/dist/index.js, since it is not marked as external in the Webpack config. This leads to the issue that if the integrator installs Watchdog version 41.3.1, it won't matter and won't be used since ckeditor5-react already includes this package.

Additionally, those packages will be duplicated in node_modules with different versions.

pomek commented 7 months ago

Thanks. I think the issue touches the Angular integration as well.

Reinmar commented 7 months ago

Internal: Fixed resolving the peer dependencies.

Is it really an internal change? Why did we need a change? It's a pity we don't have an actually issue explaining why we're changing this.

@DawidKossowski's comment mention issues with the published bundle, AFAIU. In this case, this PR is a bug fix. And should be mentioned in the changelog.

pomek commented 7 months ago

@Reinmar, I will handle it when the package is ready to release.