Make CKEditor 5 packages valid ESM

[pomek]

📝 Provide a description of the improvement

To use CKEditor 5 sources in Node without processing it by any bundler, we can mark our packages as type:module in package.json.

There's another issue that is just quasy rellated to this:

When using ckeditor from sources without a build, vitest (being esm-first project) complains about CK5 modules not being esm modules, although they seem to should be in its opinion.

Module /Users/sitecore/Sites/feaas-components/ckeditor5/node_modules/@ckeditor/ckeditor5-utils/src/dom/findclosestscrollableancestor.js:14 
seems to be an ES Module but shipped in a CommonJS package. 
You might want to create an issue to the package "@ckeditor/ckeditor5-utils" 
asking them to ship the file in .mjs extension or add "type": "module" in their package.json.

Is this something that can be resolved?

Originally posted by @Inviz in #11704 (comment)

As it is unrelated to TypeScript migration (our sources were modules before migration), I extracted the discussion to another issue.

---

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

[Inviz]

Thanks @pomek . It does seem like that would resolve the problem. It makes sense too.

[HaNdTriX]

Is there any workaround regarding this issue?
Adding

File: vite.config.ts

  test: {
    ...
+    deps: {
+      inline: [
+        'ckeditor5'
+      ]
+    }
  }

doesn't seem to work.

[Witoso]

Noticed the same, I'm not sure why inlining ckeditor5 works for our standard packages but doesn't work for RealTimeCollaborativeEditing.

For now, it's a good workaround if you're not using Collaboration:

export default {
  test: {
    deps: {
      inline: [
        "ckeditor5",
        /ckeditor5/
      ],
    },
  },
};

[filipsobol]

Adding "type": "module" changes not only how our package is consumed, but also how our tools and Node interpret and execute the code.

With "type": "module" all .js files are expected to use ESM. This causes problems with the webpack config file which uses CJS. Renaming all webpack.config.js files to webpack.config.cjs fixed this.

Another problem was caused by webpack requiring file extensions in imports in manual and automated tests.

Manual tests can be fixed by updating the getJavaScriptLoader function in @ckeditor/ckeditor5-dev-utils to return the following object:

return {
  test: /\.js$/,
  ...getDebugLoader( debugFlags ),
+  resolve: {
+    fullySpecified: false,
+  }
};

The same problem can be fixed for automated tests by adding the following objects to module.rules in lib/utils/automated-tests/getwebpackconfig.js:

{
  test: /\.js$/,
  type: 'javascript/auto',
},
{
  test: /\.js$/,
  resolve: {
    fullySpecified: false,
  }
}

Unfortunately, this doesn't solve all problems in automated testing - webpack will throw a bunch of errors that it can't find node-specific modules (e.g. Can't resolve 'stream'). More research is needed to resolve this issue.

Alternatively, we could rename all tests to .ts, as TypeScript files don't seem to suffer from this problem, but this could introduce a bunch of other issues.

For now, the easier way would probably be to add "type": "module" to package.json during the release process. If we go that route, we need to check if we also need to add "exports" to package.json to access files other than the main entry points (for example, to access the src and theme folders or package.json).

[Witoso]

@pomek what do you think?

[filipsobol]

The errors with node-specific modules might actually be caused by the setup of my local environment. I'll test this further next week, as the steps above may be enough to get it working.

[filipsobol]

I prepared two PRs:

• One to not require file extension in import paths in automated and manual tests (webpack requires them with type:module): Don't require file extension in import paths ckeditor5-dev#885.
• Another to add type:module and rename webpack.config.js to webpack.config.cjs: Add type:module to packages #14219 (depends on the first one).

TypeScript files don't need file extensions in import paths because we use "module":"es6" instead of "module":"node2016" in tsconfig.json.

I'd consider releasing alpha/beta/nightly/whatever so folks using Vite can test it. This will help us make sure we don't break anything for 39.0.0. Any thoughts? @Witoso @pomek

[pomek]

Changes in cke5-dev need tests not to break things in the future.

[filipsobol]

Changes in cke5-dev need tests not to break things in the future.

Done ✔️

[filipsobol]

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

Next week I will test our nightly releases in Vite and Vitest.

[filipsobol]

The nightly releases seem to be working fine and be a valid ESM package.

arethetypeswrong.github.io:

Before: @ckeditor/ckeditor5-core@40.2.0
After: @ckeditor/ckeditor5-core@0.0.0-nightly-20231218.0 (the warning about CJS is expected, because we only ship ESM)

publint.dev:

Before: @ckeditor/ckeditor5-core@40.2.0
After: @ckeditor/ckeditor5-core@0.0.0-nightly-20231218.0

[charlttsie]

Together with @kubaklatt, we've finished testing the nightly packages in a webpack-based setup. We tested both open-source and commercial plugins. Building editors went smoothly and we haven't encountered any issues when building with the nightly packages specifically.

I've also run CKE5 E2E tests using nightly plugin versions and no issues have been found there 🎉