Repository navigation
Conversation
Lets a pull request run the suite against an unreleased build of a package OpenProject publishes itself, while keeping that snapshot version from being merged. https://community.openproject.org/wp/OP-20422
Replaces the build committed under frontend/src/vendor with a pinned dependency, so that an editor change no longer needs build output copied into this repository. The package's own declarations make the ts-ignore on the import unnecessary. https://community.openproject.org/wp/OP-20422
Removes the docker mount for local editor development: the editor repository now copies its build over the installed package, which the frontend container already sees. A mount inside node_modules would also keep npm from replacing the package. Describes where the build now comes from. https://community.openproject.org/wp/OP-20422
This branch has not been deployed
This file contains hidden or 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
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.
Ticket
https://community.openproject.org/wp/OP-20422
What are you trying to accomplish?
Replaces the CKEditor build committed under
frontend/src/vendor/ckeditor/with the npm package@openproject/commonmark-ckeditor-build, pinned to an exact version infrontend/package.json. An editor change no longer needs build output copied into this repository, and the editor version in use is visible in the manifest.The package is introduced by opf/commonmark-ckeditor-build#140. This PR currently pins a snapshot of that PR (
0.0.0-canary-20261008132651) so the suite runs against it. The newCanary pinscheck fails on purpose until the pin is replaced with the released12.0.0.Users of Chinese (Simplified) and Portuguese (Brazil) now get a translated editor toolbar. The vendored loader asked for
zh-CN.jswhere the file iszh-cn.js, and fell back to English.What approach did you choose and why?
window.OPClassicEditor,window.OPConstrainedEditorandwindow.OPEditorWatchdog. Only where the setup service loads it from changes.loadTranslation, a static table of dynamic imports. A bundler cannot expand a templated import of a package path, and the table keeps every locale in its own lazy chunk: the production build has one chunk per locale (German is 18 kB), not one chunk with all 73.@ts-ignoreon the editor import is gone.Canary pinsrejects any0.0.0-version infrontend/package.json. It lets a PR test an unreleased build of a package OpenProject publishes itself without that snapshot being merged. It only blocks merging once it is a required check fordevandrelease/*, which is a repository setting.CKEDITOR_BUILD_DIRmount of the docker development setup is removed instead of moved. A mount insidenode_moduleswould keep npm from replacing the package, and it is not needed: the editor repository'snpm run watchcopies each build over the installed package on the host, which thefrontendcontainer already sees. Anyone who setCKEDITOR_BUILD_DIRin their.envshould setOPENPROJECT_COREin the editor repository instead.Not checked locally: the editor in a browser, and the local development loop against a running dev server. The feature specs are the behavioural test of this change.
Overlaps with #25889, which updates the vendored files this PR deletes; whichever merges second needs a rebase.
AI involvement
Merge checklist
12.0.0(Canary pinsgreen)Canary pinsmarked as required check fordevandrelease/*