Update CKEditor 34 -> 35: SyntaxError: Identifier 'global' has already been declared

[alexander-schranz]

📝 Provide detailed reproduction steps (if any)

I run into the follwoing issue:

SyntaxError: Identifier 'global' has already been declared

When running our tests with Jest after updating CKEditor 34 -> 35.

1. Run jest tests on Update CKEditor to version 35 sulu/sulu#6926

✔️ Expected result

What is the expected result of the above steps?

Tests should not fail.

❌ Actual result

What is the actual result of the above steps?

● Test suite failed to run

    /home/runner/work/sulu/sulu/node_modules/@ckeditor/ckeditor5-utils/src/dom/global.js:29
    let global;
        ^

    SyntaxError: Identifier 'global' has already been declared

      2 | import React from 'react';
      3 | import log from 'loglevel';
    > 4 | import AlignmentPlugin from '@ckeditor/ckeditor5-alignment/src/alignment';
        | ^

❓ Possible solution

If you have ideas, you can list them here. Otherwise, you can delete this section.

📃 Other details

• Browser: only jest Tests
• OS: MacOS
• First affected CKEditor version: 35
• Installed CKEditor plugins: https://github.com/sulu/sulu/blob/8ed3425211e8b72b4ac7ecc5239842408927fc42/src/Sulu/Bundle/AdminBundle/Resources/js/containers/CKEditor5/CKEditor5.js#L4-L18

---

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

[carlartz]

We are having the same issue when running our Jest Tests with CKEditor 35.

[alexander-schranz]

@carlartz I tried do debug it more, it seems like Jest itself does not like the global name for variable I changed node_modules/@ckeditor/ckeditor5-utils/src/dom/global.js:

-let global;
+let globalVar;
// In some environments window and document API might not be available.
try {
-    global = { window, document };
+    globalVar = { window, document };
}
catch (e) {
    // It's not possible to mock a window object to simulate lack of a window object without writing extremely convoluted code.
    /* istanbul ignore next */
    // Let's cast it to not change module's API.
    // We only handle this so loading editor in environments without window and document doesn't fail.
    // For better DX we shouldn't introduce mixed types and require developers to check the type manually.
    // This module should not be used on purpose in any environment outside browser.
-    globalVar = { window: {}, document: {} };
+    globalVar = { window: {}, document: {} };
}
-export default global;
+export default globalVar;

Does this also fix it in your case?

[otto10]

Same Issue with Jest when upgrading to CKEditor 35. Keeping an eye here. Following the thread.

[alexander-schranz]

@otto10 can you test if the rename would also fix it in your case?

[carlartz]

This should work, we renamed the variable locally and our tests ran successfully. Would this impact CKEditor running in our application?

[alexander-schranz]

@carlartz Thx for testing. As let normally defines scoped variables this should have zero effect if we rename the variable in that file as it is private variable. Lets wait what the CKEditor core team says about the change :).

[alexander-schranz]

@Reinmar @pomek Let me know if you need any more information and if I can do something to push this forward :)

[pomek]

cc: @arkflpc

[Reinmar]

Apologies to everyone – I forgot about this issue. Fortunately, @mremiszewski and @arkflpc corrected my mistake :) Sorry it took so long!

[Reinmar]

BTW, I wonder what's the root cause of this problem? Why does Jest have a problem with that variable? I'm a bit afraid that we fixed this particular issue but we'll hit the same problem elsewhere later on too.

[alexander-schranz]

@Reinmar jest defines a global const called global so you can not use let global somewhere. So it could maybe be catched with a eslint rule to avoid this variables named global.

Its something like this:

function jest(global) {
    // ckeditor
    let global = 2;
}

jest();

It is very similar to that you can not do let window = 'test'; inside a browser script.