LPD-67267 Overcome product limitation to add CK Editor 5 extra official custom plugins #5118
Conversation
|
CI is automatically triggering the following test suites:
|
|
Test suite sf has been triggered on http://test-1-28 |
liferay-continuous-integration
added
ci:test:sf - pending
ci:test:sf - failure
and removed
ci:test:sf - pending
labels
❌ ci:test:sf - 0 out of 1 jobs passed in 6 minutesRan com.liferay.source.formatter at released version 1.0.1543. Click here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-67267 1 Failed Jobs:For more details click here. [exec] > Task :packageRunCheckFormat
[exec] yarn run v1.22.22
[exec] \$ node-scripts check:ci
[exec]
[exec] ⚙️ Running preflight checks...
[exec]
[exec] · apps/frontend-editor/frontend-editor-ckeditor-sample-web/package.json: BAD - dependency not provided by a specific module: ckeditor5 - See https://issues.liferay.com/browse/LPS-168443
[exec]
[exec] ⚙️ Checking outdated tsconfig.json files ...
[exec] · apps/frontend-editor/frontend-editor-ckeditor-web/package.json: BAD - dependency not provided by a specific module: @ckeditor/ckeditor5-react - See https://issues.liferay.com/browse/LPS-168443
[exec]
[exec] · apps/frontend-editor/frontend-editor-ckeditor-web/package.json: BAD - dependency not provided by a specific module: ckeditor5 - See https://issues.liferay.com/browse/LPS-168443
[exec]
[exec]
[exec] ⚙️ Running TypeScript checks on modified files...
[exec] ℹ️ A total of 12 CPUs were detected: launching tsc using 12 workers
[exec] ✅ Checked apps/frontend-editor/frontend-editor-ckeditor-web
[exec] ✅ Checked apps/frontend-editor/frontend-editor-ckeditor-sample-web
[exec] ✅ Checked apps/object/object-js-components-web
[exec]
[exec] ⚙️ Running format checks on modified files...
[exec] ❌ CI checks failed.
[exec] error Command failed with exit code 1.
[exec] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[exec]
[exec] > Task :packageRunCheckFormat FAILED
[exec] Gradle build finished at 2025-10-09 13:57:56.813.
[exec]
[exec]
[exec] FAILURE: Build failed with an exception.
[exec]
[exec] * What went wrong:
[exec] Execution failed for task ':packageRunCheckFormat'.
[exec] > Process 'command '/opt/dev/projects/github/liferay-portal/build/node/bin/node'' finished with non-zero exit value 1
[exec]
[exec] * Try:
[exec] > Run with --info or --debug option to get more log output.
[exec] > Run with --scan to get full insights.
[exec] > Get more help at https://help.gradle.org.
[exec]
[exec] * Exception is:
[exec] org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':packageRunCheckFormat'.
[exec] at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda\$executeIfValid\$1(ExecuteActionsTaskExecuter.java:148) |
|
Jenkins Build:test-portal-source-format#8641 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#5118 Testray Routine:EE Pull Request Testray Build ID: 336454837Testray Importer:test-portal-source-format#8641 |
| })), | ||
| {in: path.resolve(mainEntryPoint), out: 'index'}, | ||
| ], | ||
| external: ['ckeditor5'], |
There was a problem hiding this comment.
This is technically not needed. Imports from DXP modules could work with something like
import {ButtonView, Plugin} from 'frontend-editor-ckeditor-web';but this provides better developer experience. VS code, and I assume other IDEs, can resolve from 'ckeditor5';, as they "think" it's direct dependency, without a layer in between. Resolving though 'frontend-editor-ckeditor-web' confuses VSC.
In addition, a developer can find in the package anything it expects in the official package. They don't need to be aware of the layer in between, and consider exports of frontend-editor-ckeditor-web.
There was a problem hiding this comment.
It's OK to use an external, but this external should go in the global node-scripts.config.js file. Otherwise we will start spreading stuff in several places, like we did in liferay-npm-scripts before, and the maintenance will become a nightmare.
There was a problem hiding this comment.
Isn't global node-scripts.config.js autogenerated? How would you do this? Setting needs to be added to esbuild config in the end.
We could do the same as with aliases, add a new property to node-scripts.config.js , and something like getProjectExternal() in node_scripts. There are not many modules that import from ckeditor5, it's just frontend-editor-ckeditor-web and frontend-editor-ckeditor-sample-web. In DXP it would not be an issue, as we rarely need to override default presets. But customers would need to know that they need to add this setting for a custom OSGi module.
| export * from '@ckeditor/ckeditor5-adapter-ckfinder/dist/index.js'; | ||
| export * from '@ckeditor/ckeditor5-alignment/dist/index.js'; | ||
| export * from '@ckeditor/ckeditor5-autoformat/dist/index.js'; | ||
| export * from '@ckeditor/ckeditor5-autosave/dist/index.js'; | ||
| export * from '@ckeditor/ckeditor5-basic-styles/dist/index.js'; |
There was a problem hiding this comment.
I tried
export * from 'ckeditor5';but I couldn't make it work.
There was a problem hiding this comment.
That could probably be because node-scripts wasn't able to infer the exported symbols.
| // @ts-ignore | ||
|
|
||
| import {ClassicEditor} from 'frontend-editor-ckeditor-web'; | ||
| import {CKEditor4ClassicEditor as ClassicEditor} from 'frontend-editor-ckeditor-web'; |
There was a problem hiding this comment.
Without this change, there are two ClassicEditors: one we previously had, and new one re-exported from ckeditor5.
There was a problem hiding this comment.
Wouldn't this be a breaking change from the customer perspective? Or is it that import {ClassicEditor} from 'frontend-editor-ckeditor-web'; wasn't really an API for external consumption?
There was a problem hiding this comment.
Good point. Lines are very blurry when it comes to external vs internal. I'll try to find an alternative solution.
There was a problem hiding this comment.
I pushed
export {
ClassicEditor as BaseCKEditor5ClassicEditor,
ClassicEditorUI,
ClassicEditorUIView,
} from '@ckeditor/ckeditor5-editor-classic/dist/index.js';It makes for slightly more difficult base CKE5 version upgrade, as content of that package could change, but it preserves backwards compatibility.
|
ci:test:relevant |
|
Test suite relevant has been triggered on http://test-1-39 |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#14758 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#5118 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - markocikos > liferay-frontend - PR#5118 - 2025-10-10[00:19:45] Testray Build ID: 336862092Testray Importer:test-portal-acceptance-pullrequest(master)#14758 |
|
ci:test:sf |
|
Test suite sf has been triggered on http://test-1-31 |
liferay-continuous-integration
added
ci:test:sf - pending
and removed
ci:test:sf - failure
ci:test:sf - pending
labels
|
Test suite relevant has been triggered on http://test-1-28 |
|
CI is automatically triggering the following test suites:
The pull request will automatically be forwarded to the user
|
|
Skipping previously passed test suites:
|
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#14219 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#5118 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - markocikos > liferay-frontend - PR#5118 - 2025-10-15[02:41:47] Testray Build ID: 340018391Testray Importer:test-portal-acceptance-pullrequest(master)#14219 |
|
ci:report:340018391 |
|
Test suite default has been triggered on http://test-1-31 |
liferay-continuous-integration
added
ci:test - pending
and removed
ci:test - success
labels
Build completed.Testray CSV has been generated successfully for testrayBuildID: 340018391. Job Link: generate-testray-csv Testray CSV Link: testray-results-340018391.csv |
liferay-continuous-integration
added
ci:test - success
and removed
ci:test - pending
labels
|
No unique failures, based on #5118 (comment) |
|
ci:forward:force |
|
CI is automatically triggering the following test suites:
The pull request will automatically be forwarded to the user
|
|
Skipping previously completed test suites:
|
|
Test suite default has been triggered on http://test-1-35 |
|
All required test suite(s) completed. |
|
Pull request has been successfully forwarded to brianchandotcom#166675 |
3 participants
References
What is the goal of this PR?
Enabling addition of any official CKE5 plugin through CX.
Technical notes inline
What does it look like?