feature(ckeditor): Upgrade CKEditor 5 to 37.0.1 #145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JensDallmann
force-pushed
the
feature/upgrade-ckeditor-to-37.0.0
branch
from
e3a17e7 to
6814925
Compare
mmichaelis
force-pushed
the
feature/upgrade-ckeditor-to-37.0.0
branch
2 times, most recently
from
e94b5fc to
388a5eb
Compare
Updating to CKEditor 37.0.1. As it now ships with its own typings, there is no need for typings provided by DefinitelyTypes anymore.
mmichaelis
force-pushed
the
feature/upgrade-ckeditor-to-37.0.0
branch
from
388a5eb to
458218c
Compare
@ckeditor/ckeditor5-dev-utils (dev) ... 30.5.0 ❯ 36.0.1 Also updated required peer dependency postcss: postcss (dev) .......... 8.4.16 ❯ 8.4.21 postcss-loader (dev) ... 7.0.1 ❯ 7.2.4
@ckeditor/ckeditor5-dev-webpack-plugin (dev) ... 30.5.0 ❯ 31.1.13
@babel/cli (dev) 7.18.10 ❯ 7.21.0 @babel/core 7.19.0 ❯ 7.21.4 @babel/plugin-transform-arrow-functions 7.18.6 ❯ 7.20.7 @babel/plugin-transform-computed-properties 7.18.9 ❯ 7.20.7 @babel/plugin-transform-for-of 7.18.8 ❯ 7.21.0 @babel/plugin-transform-parameters 7.18.8 ❯ 7.21.3 @babel/plugin-transform-typescript 7.19.0 ❯ 7.21.3 @babel/preset-env 7.19.0 ❯ 7.21.4 @babel/preset-typescript 7.18.6 ❯ 7.21.4 Skipped jest for now, as we have to see, if it works with recent jest-playwright.
@types/node (dev) 18.8.1 ❯ 18.15.11 @types/node (dev) 18.7.16 ❯ 18.15.11
@types/express (dev) 4.17.13 ❯ 4.17.17 express (dev) 4.18.1 ❯ 4.18.2
@types/webpack (dev) 5.28.0 ❯ 5.28.1 terser-webpack-plugin (dev) 5.3.6 ❯ 5.3.7 webpack (dev) 5.74.0 ❯ 5.78.0 webpack-cli (dev) 4.10.0 ❯ 5.0.1
@typescript-eslint/eslint-plugin (dev) 5.36.2 ❯ 5.57.1 @typescript-eslint/parser (dev) 5.36.2 ❯ 5.57.1 eslint (dev) 8.23.0 ❯ 8.37.0 eslint-config-prettier (dev) 8.5.0 ❯ 8.8.0 eslint-plugin-import (dev) 2.26.0 ❯ 2.27.5 eslint-plugin-jsdoc (dev) 39.3.6 ❯ 40.1.1 eslint-plugin-tsdoc (dev) 0.2.16 ❯ 0.2.17
css-loader (dev) 6.7.1 ❯ 6.7.3
enhanced-resolve 5.10.0 ❯ 5.12.0
prettier (dev) 2.7.1 ❯ 2.8.7
rimraf (dev) 3.0.2 ❯ 4.4.1
rxjs 7.5.6 ❯ 7.8.0 This is the same version as used in CoreMedia Studio.
style-loader (dev) 3.3.1 ❯ 3.3.2
ts-loader (dev) 9.3.1 ❯ 9.4.2
tslib 2.4.0 ❯ 2.5.0
typedoc (dev) 0.23.14 ❯ 0.23.28
xml-formatter 2.6.1 ❯ 3.3.2
typescript 4.8.2 ❯ 4.9.5
playwright (dev) 1.25.2 ❯ 1.32.2 playwright-core (dev) 1.25.2 ❯ 1.32.2
@types/jest (dev) 28.1.8 ❯ 29.5.0 babel-jest 28.1.3 ❯ 29.5.0 jest (dev) 28.1.3 ❯ 29.5.0 jest-circus (dev) 28.1.3 ❯ 29.5.0 jest-config (dev) 28.1.3 ❯ 29.5.0 jest-each (dev) 28.1.3 ❯ 29.5.0 jest-environment-jsdom 28.1.3 ❯ 29.5.0 jest-environment-node (dev) 28.1.3 ❯ 29.5.0 jest-playwright-preset (dev) 2.0.0 ❯ 3.0.1 jest-runner (dev) 28.1.3 ❯ 29.5.0
mmichaelis
force-pushed
the
feature/upgrade-ckeditor-to-37.0.0
branch
from
eb67a48 to
b34c656
Compare
After updating xml-formatter, we need to update the usage accordingly.
16:45 error This assertion is unnecessary since it does not change the type of the expression @typescript-eslint/no-unnecessary-type-assertion
9:36 error Replace `typeof·knownNamespacePrefixes` with `(typeof·knownNamespacePrefixes)` prettier/prettier
…tion. Github runner sometimes are faster, sometimes are slower. We should not cause problems to us due to the variation. Timeouts are only a safety net to not spend too much if something does not progress (e.g. playwright tests).
…h ts Types are referenced for local development (code completion) with the index.ts but after publication it must reference the index.d.ts.
We decided to switch to building to src/ as this seems to be common practice and does not trigger error-prone auto-complete when importing in IDEs, that sometimes suggest the dist/ folder as the folder to add to the import statement. Also, we hope, that we get closer to the recommended artifact setup, as it ships with CKEditor 5 core plugins. As first step, adapted `rimraf` to respect the new structure, while still providing an portal to the old setup (just to clean up our own workspaces, or while still migrating to new structure). Also, removed `rimraf` from dependencies but now aligned all invocations to `npx`, as we already did for removing node_modules.
Get rid of rimraf dependency.
As next step, prior to building to "src/", we have to ignore these files in `.gitignore`. Fortunately, we migrated our example app also to TypeScript long before, so that there are now no `*.js` files left, that must not be ignored.
Also, we need/should ignore these files when linting, as reports are not relevant to us.
Also, we need/should ignore these files when linting, as reports are not relevant to us.
As we don't have a dist/-folder anymore, we also don't need to copy the theme files (hope, that theming still works).
Remove copyfiles.
Change reference from dist/ to src/ folder for artifacts.
Webpack still builds to dist/. In CKEditor 5, it builds to build/ instead. Don't see any gain in keeping it that way, thus, reverting to dist/ again for example application.
Removing outDir configuration in tsconfig.json, so that now all files are built into the src/ folder, as it is the default.
For node support, we need to specify the JS files as `main`. While CKEditor 5 comes with publish-tooling to replace this dynamically, we decide to add it as extra publishConfig, instead.
Needed to revert. False assumption was, that dependency is not required using npx. Seems, as if this assumption is wrong. Switched back to explicit (dev) dependency.
Since rimraf 4.x, --glob option is required to handle globs. And forgot to "duplicate" pnpm call in clean-task. Fixed.
Need to add more "glob" to it for nested paths.
Provide similar pattern as used in CKEditor 5 workspace, thus, declaring svgs in typings/ folder.
…sted attributes objects. We may want a better solution but as it not breaking we can still improve later.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Main Purpose
Upgrade CKEditor 5 to 37.x.
CKEditor 37.x ships with its own typings. Thus, we do not need to depend on sometimes outdated, sometimes not fully compatible typings provided at DefinitelyTyped.
The goal of this PR is to update to CKEditor 5 37.x as well as to benefit from new typings and subsequently ease integration into CoreMedia Studio from a developer perspective (thus, not having to maintain typing dependencies along with CKEditor dependencies).
Full Changelog
While some more third-party updates are directly related to the CKEditor 5 update, we also decided to update all third-party dependencies where feasible.
Dependency Updates
Types: ⏫ Major; 🔼 Minor; 🩹 Patch
@babel/cli@babel/core@babel/plugin-transform-arrow-functions@babel/plugin-transform-computed-properties@babel/plugin-transform-for-of@babel/plugin-transform-parameters@babel/plugin-transform-typescript@babel/preset-env@babel/preset-typescript@ckeditor/ckeditor5-*@ckeditor/ckeditor5-dev-utils@types/express@types/jest@types/jest@types/node@types/node@types/webpack@typescript-eslint/eslint-plugin@typescript-eslint/parserbabel-jestcss-loaderenhanced-resolveeslint-config-prettiereslint-plugin-importeslint-plugin-jsdoceslint-plugin-tsdoceslintexpressjest-circusjest-configjest-eachjest-environment-jsdomjest-environment-nodejest-playwright-presetjest-runnerjestplaywright-coreplaywrightpostcss-loaderckeditor5-dev-utilspostcssckeditor5-dev-utilsprettierrimrafrxjsstyle-loaderterser-webpack-plugints-loadertslibtypedoctypescriptwebpack-cliwebpackxml-formatterDependency To-dos
@ckeditor/ckeditor5-dev-webpack-pluginis marked as deprecated. We should review the deprecation and possibly adapt.Required Changes
Despite some changes triggered by new ESLint and Prettier versions, we also had to adapt to the new API:
Mixin Approach: CKEditor 5 now uses one of the suggested approaches for mixins in TyeScript. As this also fixes corresponding linter issues, we applied these adaptations as well by extending, for example,
ObservableMixin.Prefer
index.tsimport: As stated by CKSource their public API contract is now provided byindex.ts. Any usages of API not referenced here are subject to change, i.e., may be changed without further notice between CKEditor 5 upgrades.We applied these changes and notes, where we had to violate them (and reported) these usages to CKSource, so that in the long run we may come to a pure public API approach.
For the CKEditor 5 plugins provided by this workspace, we also decided to ship an
index.ts, but the contract may need further review. Note, that for augmentation support (see below), you need to useindex.ts.Introduce
augmentation.ts: As convenient pattern introduced for CKEditor 5 and meanwhile part of the standard plugin creation wizard, we addedaugmentation.ts, so that, for example, plugins or commands retrieved viaeditorinstance automatically provide proper typing (not relevant for constructor-based access inPluginsMap, though).Remove
@ckeditor/ckeditor5-dev-webpack-plugin: As building the example application failed, we had to replace our previous webpack integration and align it with the way, how CKEditor 5 packs their editor builds (such as for classic editor). Having this, we got rid (and had to get rid) off the deprecated dependency@ckeditor/ckeditor5-dev-webpack-plugin.Possibly Required Studio Integration Change: This change may need to be adapted for the CoreMedia Studio Workspace.
Todos
Identify FontMapper Test Issues
Test had to be disabled for now (
it.skip). Already identified, that Jest, Jest-Playwright and Playwright updates alone did not trigger the issue. Current debugging shows, that the issue is that in headless mode, the clipboard is not updated as expected, and, thus, subsequently the paste action fails. Works in headed mode, though.We may decide to do this as follow-up task, but we should do it, as the test is an important integration test for CKEditor updates.
Analyse and Decide on Typing Issues
There are still some TODOs, we should review, try to find alternatives (like new
ConfigAPI with generics).Announce CKEditor 5 Walk-Throughs
We may want to announce the new "walk-through" section as "lightweight" documentation approach.
See Also
TypeScript Migration Issues solved with 37.x:
Follow-Up issues related to our upgrade:
Further Reading: