Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mix of ECMAScript modules & CommonJS causes vite & vitest issues #14097

Closed
HaNdTriX opened this issue May 10, 2023 · 6 comments
Closed

Mix of ECMAScript modules & CommonJS causes vite & vitest issues #14097

HaNdTriX opened this issue May 10, 2023 · 6 comments
Labels
domain:integration-dx This issue reports a problem with the developer experience when integrating CKEditor into a system. squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@HaNdTriX
Copy link

HaNdTriX commented May 10, 2023

TLTR

Using @ckeditor/ckeditor5-real-time-collaboration breaks the build system of vitest (or vite) because ECMAScript modules and CommonJS Modules are used in the same NPM Package.

📝 Provide detailed reproduction steps (if any)

  1. Create a vite project
  2. Add vitest as testing library
  3. Add the following test
import { it, expect } from 'vitest'

import RealTimeCollaborativeEditing from '@ckeditor/ckeditor5-real-time-collaboration/src/realtimecollaborativeediting'

it('allows to import RTC-Tooling', async () => {
  console.log(RealTimeCollaborativeEditing)
  expect(true).toBe(true)
})

✔️ Expected result

The test should pass

❌ Actual result

Module <user-dir>/node_modules/@ckeditor/ckeditor5-core/src/index.js:8 seems to be an ES Module but shipped in a CommonJS package. You might want to create an issue to the package "@ckeditor/ckeditor5-core" asking them to ship the file in .mjs extension or add "type": "module" in their package.json.

As a temporary workaround you can try to inline the package by updating your config:

// vitest.config.js
export default {
  test: {
    deps: {
      inline: [
        "@ckeditor/ckeditor5-core"
      ]
    }
  }
}

The log above suggests to inline @ckeditor deps to temporarily fix the issue (#13673), nevertheless this would not work, because the @ckeditor/ckeditor5-real-time-collaboration module is mixing ESM & CommonJS in the same npm package.

FAIL  tests/unit/NavigationBar.spec.ts [ tests/unit/NavigationBar.spec.ts ]
Error: require() of ES Module <user-dir>/node_modules/ckeditor5/src/utils.js from <user-dir>/node_modules/@ckeditor/ckeditor-cloud-services-collaboration/src/websocketgateway/websocketgateway.js not supported.
Instead change the require of utils.js in <user-dir>/node_modules/@ckeditor/ckeditor-cloud-services-collaboration/src/websocketgateway/websocketgateway.js to a dynamic import() which is available in all CommonJS modules.
 ❯ Object.<anonymous> node_modules/@ckeditor/ckeditor-cloud-services-collaboration/src/websocketgateway/websocketgateway.js:23:380
 ❯ async <user-dir>/node_modules/@ckeditor/ckeditor5-real-time-collaboration/src/realtimecollaborativeediting/websocketgateway.js:3:31
 ❯ async <user-dir>/node_modules/@ckeditor/ckeditor5-real-time-collaboration/src/realtimecollaborativeediting/realtimecollaborationclient.js:5:31
 ❯ async <user-dir>/node_modules/@ckeditor/ckeditor5-real-time-collaboration/src/realtimecollaborativeediting.js:2:31
 ❯ async <user-dir>/tests/unit/NavigationBar.spec.ts:2:31


Serialized Error: {
  "code": "ERR_REQUIRE_ESM",
}

❓ Possible solution

  1. Convert all ckeditor5 deps to esm packages by adding "type": "module", to their package.json files
  2. Decide wich format the @ckeditor/ckeditor5-real-time-collaboration should be in (ECMAScript modules or CommonJS) and convert the files to the corresponding format.

📃 Other details

  • First affected CKEditor version: 37.1.0
  • Installed CKEditor plugins:
    • "@ckeditor/ckeditor5-real-time-collaboration": "37.1.0"

Related Issue


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

@HaNdTriX HaNdTriX added the type:bug This issue reports a buggy (incorrect) behavior. label May 10, 2023
@Witoso
Copy link
Member

Witoso commented May 12, 2023

Thanks for the issue, I tried to find workarounds but failed. The @ckeditor/ckeditor5-real-time-collaboration is obfuscated, and is CJS while all other packages are ESM but not marked as such. Vitest is working when you use our packages with inlining but not for ckeditor5-real-time-collaboration.

Two actions on our side:

  1. @pomek could you take a look at our internal packages? I noticed everything works ok (with inlining) with for example internal SlashCommand but not for RealTimeCollaborationEditing. What's the difference (we can move the discussion to internal channels if needed)?
  2. We will have a team discussion on type:module.

@Witoso Witoso added squad:platform Issue to be handled by the Platform team. squad:core Issue to be handled by the Core team. domain:integration-dx This issue reports a problem with the developer experience when integrating CKEditor into a system. labels May 12, 2023
@HaNdTriX
Copy link
Author

HaNdTriX commented May 15, 2023

I am not sure if this is related but it seems @ckeditor/ckeditor5-real-time-collaboration is requiring the wrong version of @ckeditor/ckeditor-cloud-services-collaboration (46.0.0 instead 37.1.0? #semver)

File: yarn.lock

...
"@ckeditor/[email protected]":
  version "37.1.0"
  resolved "https://registry.yarnpkg.com/@ckeditor/ckeditor5-real-time-collaboration/-/ckeditor5-real-time-collaboration-37.1.0.tgz#047effac4ae604bb192a6f61b9f328c3dad67ae0"
  integrity sha512-fmOHzWKCfB+J9yWboyUEmyM1JoCYzZvDvmfgw902M3+o4OViI9QN1f7ocDJSV89eL0BXALX0rIckz1H1ZRSGlA==
  dependencies:
    "@ckeditor/ckeditor-cloud-services-collaboration" "^46.0.0"
    "@ckeditor/ckeditor5-cloud-services" "^37.1.0"
    "@ckeditor/ckeditor5-operations-compressor" "^37.1.0"
    ckeditor5 "^37.1.0"
    ckeditor5-collaboration "^37.1.0"
    lodash-es "^4.17.11"
...

This Issue is blocking us from using vite and vitest. Any Quick Fix would be appreciated.

@Witoso
Copy link
Member

Witoso commented May 15, 2023

@HaNdTriX we will investigate this but I cannot promise or offer any quick fix at the moment.

I am not sure if this is related but it seems @ckeditor/ckeditor5-real-time-collaboration is requiring the wrong version of @ckeditor/ckeditor-cloud-services-collaboration (46.0.0 instead 37.1.0? #semver)

Not related, this package has a different versioning.

@lslowikowska lslowikowska added the support:2 An issue reported by a commercially licensed client. label Jun 15, 2023
@Witoso
Copy link
Member

Witoso commented Jul 24, 2023

In relation to this issue, we released two alpha versions that properly mark packages as ESM modules. But this doesn't fix the Vitest issues. Other issues appear in relation to testing the editor in the node environment. We stumbled on similar problems in Jest and the same problems may appear for Vitest:

(...) it runs tests in Node.js with the use of JSDOM. JSDOM is not a complete DOM implementation and while it's apparently sufficient for standard apps, it's not able to polyfill all the DOM APIs that CKEditor 5 requires.

For now, we recommend switching the environment for Vitest to test the editor in the browser:

import { defineConfig } from 'vitest/config';

export default defineConfig({
  test: {
    browser: {
      enabled: true,
      name: 'chrome',
      provider: 'webdriverio'
      // or "name: 'chromium'" and "provider: 'playwright'"
    }
  },
});

The tests will work without the need to update to alpha ESM packages.

@pomek pomek removed the squad:platform Issue to be handled by the Platform team. label Jul 25, 2023
@filipsobol
Copy link
Member

filipsobol commented Dec 15, 2023

Changes to make our packages valid ESM are merged and will be released in January.

You can follow the updates in #13673

@filipsobol
Copy link
Member

A fix will be available in the next release.

You'll be able to remove the workaround from the vite.config.js file, but automated testing of CKEditor will still need to be done with a real browser as shown by @Witoso.

@Witoso Witoso added this to the iteration 70 milestone Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:integration-dx This issue reports a problem with the developer experience when integrating CKEditor into a system. squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

5 participants