Ckeditor5: Docs builder uses its dependency when building snippets

Created on 24 Aug 2020  ยท  13Comments  ยท  Source: ckeditor/ckeditor5

๐Ÿ“ Provide detailed reproduction steps (if any)

A problematic feature is marked plugin which is used by ckeditor5-markdown-gfm plugin.

โœ”๏ธ Expected result

The dependencies of the the docs builder should not shadow dependencies of editor plugins (if possible).

โŒ Actual result

See #7906.


If you'd like to see this fixed sooner, add a ๐Ÿ‘ reaction to this post.

dx dev markdown-gfm devops bug

All 13 comments

ps.: To unblock release process I've added marked as CKEditor5 dependency in 9e71179036a78f15c1bcbfa2801acf276359adba . This hinders the issue but in long run is not sustainable. See

TODO: Check webpack configuration in the snippet adapter. Probably, it does not limit webpack's search path (I don't remember how's that option named โ€“ it's defining where webpack searches for node_modules) to just ckeditor5 but allows it leaking outside, thus, reading from documentation builder.

yarn why v1.22.4
[1/4] Why do we have the module "marked"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "_project_#jsdoc" depends on it
   - Hoisted from "_project_#jsdoc#marked"
info Disk size without dependencies: "194KB"
info Disk size with unique dependencies: "194KB"
info Disk size with transitive dependencies: "194KB"
info Number of shared dependencies: 0
=> Found "@ckeditor/ckeditor5-markdown-gfm#[email protected]"
info This module exists because "_project_#@ckeditor#ckeditor5-markdown-gfm" depends on it.
info Disk size without dependencies: "260.5KB"
info Disk size with unique dependencies: "260.5KB"
info Disk size with transitive dependencies: "260.5KB"
info Number of shared dependencies: 0
=> Found "@ckeditor/jsdoc-plugins#[email protected]"
info Reasons this module exists
   - "_project_#umberto#@ckeditor#jsdoc-plugins#jsdoc" depends on it
   - Hoisted from "_project_#umberto#@ckeditor#jsdoc-plugins#jsdoc#marked"
info Disk size without dependencies: "86KB"
info Disk size with unique dependencies: "86KB"
info Disk size with transitive dependencies: "86KB"
info Number of shared dependencies: 0
Done in 1.07s.

Looks like we cannot do too much here. I'll try to reconfigure (somehow) snippet adapter. Maybe it can look for packages' dependencies (packages/*/node_modules) first.

We could update Umberto's dependencies :see_no_evil: :trollface:

It comes from jsdoc. Umberto does not require it.

After modifying paths to modules in webpack configuration, docs are fine.

// Configure the paths so building CKEditor 5 snippets work even if the script
// is triggered from a directory outside ckeditor5 (e.g. multi-project case).
resolve: {
    modules: [
        // See: #7916.
        path.resolve( __dirname, '..', '..', 'packages', 'ckeditor5-markdown-gfm', 'node_modules' ),
        ...getModuleResolvePaths(),
    ]
},

But I'm not sure about it. The problem can occur with other dependencies across all packages. Should we leave it as it is (the snippet above), or try to be future-proof (the snippet below)?

// Configure the paths so building CKEditor 5 snippets work even if the script
// is triggered from a directory outside ckeditor5 (e.g. multi-project case).
resolve: {
    modules: [
        path.resolve( __dirname, '..', '..', 'packages', 'ckeditor-cloud-services-core', 'node_modules' ),
        path.resolve( __dirname, '..', '..', 'packages', 'ckeditor5-adapter-ckfinder', 'node_modules' ),
        // ...
        path.resolve( __dirname, '..', '..', 'packages', 'ckeditor5-word-count', 'node_modules' ),
        ...getModuleResolvePaths(),
    ]
},

I decided to implement the second option.

OK, so to recap:

  • There are 3 marked installed โ€“ one is the dependency of ckeditor5-markdown-gfm, the other of umberto and the last one of jsdoc.
  • umberto and jsdoc and ckeditor5-markdown-gfm are dependencies of ckeditor5 so if we run yarn install inside ckeditor5 then marked must be installed 3 times (because each of the consumers needs a different version).
  • One marked is probably installed directly inside ckeditor5/node_modules. The other inside ckeditor5/packages/ckeditor5-markdown-gfm/node_modules and another one somewhere inside ckeditor5/node_modules/umberto/node_modules.

Am I right?

If so, we indeed have a problem because snippets are right now built in the context of ckeditor5.

However, the problem is IMO in this code: https://github.com/ckeditor/ckeditor5/blob/785302b9d4b73bd9cafe9f2b1f0a320278e57583/scripts/docs/snippetadapter.js#L467-L472

Because it says โ€“ whenever you do require( 'marked' ) look for it FIRST in ckeditor5/node_modules and only if you didn't find it there, use the default algorithm. The default algorithm would travers the tree from the place where require() was used up the tree to the root. So it would first check ckeditor5/packages/ckeditor5-markdown-gfm/node_modules.

Therefore, IMO, the solution should be to remove the first line from getModuleResolvePaths(). The question is โ€“ why was it used there?

The question is โ€“ why was it used there?

Not an answer, but I found the initial commit of this change: https://github.com/ckeditor/ckeditor5/commit/701ee5b9a79b06a6c6e235bbd6c1f4baa3bc1273.

I'm assuming it's related to multi-repo architecture (in order to resolve links from _linked_ packages).

OK, so most likely it was needed when ckeditor5 and its packages were cloned inside CF's project to also include CF's deps. I suppose it's not needed anymore in our normal env, but please also check the documentation builder (IDK how the setup looks there).

Unfortunately, it does not work. I tried also a trick with changing cwd. Docs for v5 will be built but the documentation builder will throw errors.

In the proposed PR (#8215), I've added an option to look for paths to packages inside the external/ directory.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

wwalc picture wwalc  ยท  3Comments

Reinmar picture Reinmar  ยท  3Comments

msamsel picture msamsel  ยท  3Comments

Reinmar picture Reinmar  ยท  3Comments

MansoorJafari picture MansoorJafari  ยท  3Comments