Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
CKEditor is using umberto for generating their documentation. It's an internal tool which they are using. It would be useful to improve our documentation by reference relevant parts of CKEditor code. Ideally, when code from CKEditor is being referenced in our code, it is something that IDEs are able to figure out, and it would be possible to navigate using their shortcuts to the relevant part of the CKEditor code.
Proposed resolution
- Create an intermediate type that maps to the CKEditor 5 module:
/** * @typedef {module:engine/model/element} module:engine/model/element~Element */
- Provide CLI tool that automatically generates this mapping for all types that can be mapped
Steps for testing
- In the IDE, associate the file type
*.jsdoc
as aJavaScript file. to take advantage of the mapping provided by the generated .jsdoc file. core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediaediting.js
, and see if your IDE recognizesmodelElement
argument type. When the argument type is recognized, the IDE should be able to provide guidance on what are the accessible properties of that type.- If this does not work, it is possible your IDE is not including
node_modules/@ckeditor5
is in its code completion index. For example, in PHPStorm, it may be necessary to explicitly mark the directory as "Not Excluded"
Comment | File | Size | Author |
---|---|---|---|
#27 | 3248423-27-9.3.x.patch | 97.55 KB | Wim Leers |
#20 | 3248423-20.patch | 97.27 KB | bnjmnm |
|
Issue fork drupal-3248423
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
Wim LeersComment #3
Wim LeersComment #5
Wim LeersI wonder if this blocks #3248430: Improve Drupal.ckeditor5 documentation? 🤔
Comment #6
nod_So I have a way to make documentation show up in PhpStorm but it's not pretty.
File | Settings | Languages & Frameworks | JavaScript | Libraries
, add a new library, which points to the foldernode_modules/@ckeditor
@param {module:engine/model/element~Element} modelElement
by@param {module:engine/model/element} modelElement
so that the default export is used for the content of the this.I don't think we can have a working out of the box experience for IDE autocompletion.
Comment #7
nod_This is specific to jetbrains IDE but they're broadly used in the community so here goes a list of relevant issues about jsdoc & modules in webstorm:
Comment #8
nod_Spent some time with lauriii looking at the issue.
What works in PhpStorm:
*.jsdoc
as javascript filesThis takes advantage of the fact that referencing
{module:engine/model/element}
makes the autocompletion work while keeping the same declaration that ckeditor folks have in their code.I'll work on automatically generating a mapping for ckeditor types so we can provide a
ckeditor5.types.jsdoc
file that will make IDE autocompletion work as intended.Comment #9
nod_Comment #11
nod_yolo solution, need more docs to explain what's happening but should be somewhat helping.
Comment #12
nod_tweaked the regex, only 86 files ignored from the ckeditor source. I think I caught all the export default, and @interface alias declarations in the code.
The files that don't match are files with several exports.
Comment #13
lauriiiI tested some common classes that we use in our plugins such as writers, elements and there seems to be pretty good coverage. Before this patch, I'd say 0% of CKEditor types were recognized by PHPStorm. After this patch, maybe 70-80% are, which is already a huge DX improvement. Luckily the approach on this issue doesn't require anything special from when we document types in our code, meaning that all of the magic is behind the scenes and could be changed easily. This means that we could be easily iterate on this in future if we come up with new ideas what could improve this.
Comment #14
nod_Also we can just get rid of it altogether when IDEs become "smart enough" :)
Comment #15
bnjmnmComment #16
lauriiiComment #17
bnjmnmComment #18
bnjmnmThis is a clever solution. Cksource has a proprietary doc generation standard (Umberto) that IDEs such as PHPStorm do not currently support. The ckeditor5-types-
documentation.js script scans the contents of
node_modules/@ckeditor5
for Umberto definitions. These definitions are used to dynamically generate a file -ckeditor5.types.jsdoc
- that uses JSDoc to map IDE-understood typedefs to their actual definitions in CKEditor5.ckeditor5.types.jsdoc
needs to be recognized as a JavaScript file in order to work so the IDE knows to parse the mapping as JSDoc.This is a big DX improvement. It has zero impact on any runtime code so introducing these improvements now introduces no risk or debt. It can be improved on later if we wish, or we can just enjoy the considerable improvements introduced here.
Will commit once tests complete after the rebase I just performed.
Comment #19
lauriiiI guess something worth noting is that the syntax CKEditor is using is standardized in JSDoc 3 (ES 2015 modules and namepaths). It just isn't supported by most of the tooling yet (including IDEs). However, CKSources proprietary doc generation tool does support it, and hopefully other tools will also provide support in the future, which will allow us to remove the mapping. The workaround that we apply is that we remove the inner member in the mapping from modules that don't need it because they only have one inner member. Because of that, this solution cannot handle 100% of CKEditor code base.
Comment #20
bnjmnmAlthough it's likely quite safe for a number of reasons, I'm reluctant to Gitlab-merge an MR that needs rebasing until after the tests have fully run post-rebase.
Unfortunately, I keep missing the window between tests passing and not needing another rebase (the window often closes while tests run), so here's a patch of the current MR that I'll be using to commit this to D10.
Comment #22
bnjmnmCommitted to 10.0.x, very nice addition.
Will see if 9.4.x is cherry pickable.
Comment #24
bnjmnmCherry picked to 9.4.x.
Comment #26
catchRe-opening for 9.3.x backport.
Comment #27
Wim LeersPatch from local chery-pick.
Comment #29
lauriiiCommitted 5e7f9e0 and pushed to 9.3.x. Thanks!