New CKEditor5 installation methods break tests: "domDocument.createElement is not a function"

[stonebk]

📝 Description

I'm trying to integrate CKEditor5 into my project with the new installation methods, and I'm seeing a lot of existing tests fail due to a transitive CKEditor5 dependency. Basically, we have tests that don't depend on CKEditor5 directly, but have deeply nested children that consume CKEditor5. These children are never rendered in the test because we are shallow rendering the parent.

Our tests are currently running with @jest-environment node because it's a lot faster than @jest-environment jsdom. CKEditor5, however, assumes a global document object which breaks all tests that have CKEditor5 somewhere in the dependency tree.

✔️ Expected result

Test runs without a TypeError.

❌ Actual result

  ● Test suite failed to run

    TypeError: domDocument.createElement is not a function

    > 1 | import { Autoformat, DecoupledEditor, EditorConfig, Essentials, Paragraph } from 'ckeditor5';
        | ^
      2 |
      3 | import ContentState from './plugins/ContentState/ContentStatePlugin';
      4 |

      at createElement (../../common/temp/node_modules/.pnpm/@ckeditor+ckeditor5-engine@43.2.0/node_modules/@ckeditor/ckeditor5-engine/src/view/filler.ts:72:31)
      at Object.BR_FILLER (../../common/temp/node_modules/.pnpm/@ckeditor+ckeditor5-engine@43.2.0/node_modules/@ckeditor/ckeditor5-engine/src/view/domconverter.ts:54:23)
      at Object.require (../../common/temp/node_modules/.pnpm/@ckeditor+ckeditor5-core@43.2.0/node_modules/@ckeditor/ckeditor5-core/dist/index.js:2:1)
      at Object.require (../../common/temp/node_modules/.pnpm/@ckeditor+ckeditor5-adapter-ckfinder@43.2.0/node_modules/@ckeditor/ckeditor5-adapter-ckfinder/dist/index.js:1:1)
      at Object.require (../../common/temp/node_modules/.pnpm/ckeditor5@43.2.0/node_modules/ckeditor5/dist/ckeditor5.js:1:1)

❓ Possible solution

These tests can be fixed simply by switching from @jest-environment node to @jest-environment jsdom, but this slows things down even though CKEditor5 is not always being used. Also, not all imports from CKEditor5 require a global document, but it seems like with the new installation method, any import will now require a browser environment. Another possible workaround is to just go back to direct imports rather than importing everything from ckeditor5.

A possible solution could be to not access the document object in the global scope. For example, in domconverter.ts, the global document generates some constant variables at the top level:

const BR_FILLER_REF = BR_FILLER( global.document ); // eslint-disable-line new-cap
const NBSP_FILLER_REF = NBSP_FILLER( global.document ); // eslint-disable-line new-cap
const MARKED_NBSP_FILLER_REF = MARKED_NBSP_FILLER( global.document ); // eslint-disable-line new-cap

Instead, these could potentially be created in the constructor when a DomConverter is instantiated and wouldn't require the document object unless people are actually consuming the DomConverter class.

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[Witoso]

Hi!

This would mean tweaking quite many internals of our engine (meaning it's not only what you pointed). I assume this problem is related to tree shaking (#16395) and possibly barrel files. It's possible that optimized imports will work better, but I'm not sure, @filipsobol?

Another possible workaround is to just go back to direct imports rather than importing everything from ckeditor5.

I would advise not to do that via src, imports from src will be deprecated, optimized build path may be more promising for you.

Testing the editor outside a browser is a thing we don't support, even jsdom is problematic. Have you considered mocking the values?

[filipsobol]

It's possible that optimized imports will work better, but I'm not sure

It's unlikely, but worth trying.

[stonebk]

Thank you for your response @Witoso!

This would mean tweaking quite many internals of our engine (meaning it's not only what you pointed). I assume this problem is related to tree shaking (#16395) and possibly barrel files.

Correct me if I'm wrong, but I think this would still be a problem because tree-shaken code is only marked for deletion -- tests would need to run on production/minified code for the code to actually be removed.

I would advise not to do that via src, imports from src will be deprecated, optimized build path may be more promising for you.

Sorry, what I meant by direct imports is what you are calling optimized imports. Maybe I misunderstood, but I thought one of the goals of #15502 was to replace individual imports with a single ckeditor5 import (like you said, this does depend on tree-shaking). Are you trying to move away from optimized imports, or are they always going to be an optimization strategy?

Testing the editor outside a browser is a thing we don't support, even jsdom is problematic. Have you considered mocking the values?

Mocking is definitely an option, and probably something I will explore next -- I just wanted to reach out and get your general thoughts before I started making changes. If you don't think my use-case is valid and that this is expected behavior for the ckeditor library, then I'll go a different route.

[Witoso]

I think this would still be a problem because tree-shaken code is only marked for deletion -- tests would need to run on production/minified code for the code to actually be removed.

I think that depends on your bundler/build process for tests. Maybe mentioning tree-shaking was too big a shortcut, and it is more of an effect than a cause. As you mentioned, the code leaks the need for browser's globals. Importing from indexes will pollute the scope.

Maybe I misunderstood, but I thought one of the goals of #15502 was to replace individual imports with a single ckeditor5 import (...)

The main goal was to reduce the forced webpack dependency, and also improve DX of installing tons of packages. But we realized at some point, there's going to be a need for something for more advanced users, for optimizing every byte, as 100% treeshakable code is an utopian dream in the project of our size. Some libraries call it cherry-picking, and optimized path will stay with us for sure.

Currently, we monitor tree-shaking size, and it will be brought down in the future, but engine is near the end our list, as we assumed that every editor instance needs the core internals. But the case you mentioned: import, don't render, may help us revisit the priorities list (@filipsobol, @Reinmar). For now, the mocking is the safest path in a non-browser env.

[stonebk]

Sounds good, thank you @Witoso!