Preserve es modules in babel config

[fnlctrl]

For users installing from npm, they already have a bundler that can handle es modules.
For users that want to run quill in nodejs directly, it's currently not possible due to side effects calling DOM

By preserving modules, it allows third party dependencies (e.g. lodash-es) to be tree shaken. Currently lodash-es is fully bundled by users because of es modules transpiled to require() https://www.npmjs.com/package/quill/v/2.0.0-rc.1?activeTab=code

[luin]

Should we add "type": "module" in package.json?

[fnlctrl]

@luin Adding "type": "module" seems to break webpack (build/dev-server). We should be fine without it since we don't need it to be run in node. I added "sideEffects":false to potentially help with treeshaking

[luin]

I assume quill.js has side effects as it registers formats?

[fnlctrl]

They won't be discarded by bundlers (webpack/vite) as far as I know. Their tree shaking is very conservative and requires explicit /*#__PURE__*/ markings for these side effects (function calls, assignments)

Edit: found the relavent documentation from webpack

[luin]

Thanks! I removed sideEffects field. I did some tests and it does remove the imports that are not used (but have side effects). We can figure out other approaches if the size is an issue.

[alecgibson]

@luin @fnlctrl Shipping without "type": "module" breaks builds that run this on Node.js, for example test suites running through jsdom in Node.js:

import {AlignClass} from '@reedsy/quill/formats/align.js';
        ^

SyntaxError: The requested module '@reedsy/quill/formats/align.js' does not provide an export named 'AlignClass'

[alecgibson]

To get this to work on Webpack with "type": "module", you need to set:

{
  test: /\.m?js/,
  resolve: {
    fullySpecified: false
  }
}

[alecgibson]

Also I'm not personally convinced with shipping this as ESM-only. Feels massively breaking.

[alecgibson]

Super simple repro repo for the issue. Just run npm test:

https://stackblitz.com/edit/stackblitz-starters-1s5ssr?file=index.spec.js

[luin]

@alecgibson Good point on the jsdom issue! I thought users won't run Quill in Node.js. Do you have any suggestions for #4007?

[alecgibson]

@luin there's a good write-up and quick-fix solution here: https://www.npmjs.com/package/default-import

The basic issue is that Quill mixes default and named exports, which doesn't make ESM happy.

The short-term solution is that ESM consumers can use the default-import library:

import _Quill from 'quill';
import { defaultImport } from 'default-import';

const Quill = defaultImport(_Quill);

const quill = new Quill('#editor', {
  theme: 'snow',
});

Here's a fixed fork of the Stackblitz in #4007 : https://stackblitz.com/edit/esbuild-template-hwaow1?file=package.json

The long-term solution is to stop mixing named and default exports in Quill.

[alecgibson]

(We'd also need to revert this change, which breaks Node.js usage)

[luin]

@alecgibson Will take a look! Probably we can revert the change and also export Quill as a named export (while keep the default export for backward compatibility) and advocate the named export usage. WDYT?

import { Quill } from 'quill';
import { Quill } from 'quill/core';

The only downside I can see is Quill.Quill is set. But I think that should be fine.

[alecgibson]

@luin I've not tested it, but I imagine that's a sensible solution; we'd ideally do this everywhere, because we mix default/named exports in quite a few places.

[luin]

Yeah actually I'm thinking about expose all things from the main file, so instead of

import Block from 'quill/blots/block';

Users could do

import { Block } from 'quill';
import { Block } from 'quill/core';

We will still keep import Block from 'quill/blots/block'; though for backward compatibility.

[luin]

@alecgibson Just wanted to double-check, v1.3.7 and v2.0.0-dev.4 are both use ESM format and looks like we only changed it to CJS in 2.0.0 beta version. Is that something that you had fixed in your fork version before 2.0.0 beta?

[alecgibson]

@luin I'm not sure I understand what you mean?

[luin]

@alecgibson https://www.npmjs.com/package/quill?activeTab=code 1.3.7 and 2.0.0-dev.4 also use ESM import, did you find the same issue with those versions?

[alecgibson]

@luin we haven't run into this ourselves, but:

• we only moved some server-side code to ESM very recently
• our client-side code is still in CJS, with both TypeScript and Webpack in the toolchain between "raw" source and our transpiled code

Maybe I was wrong about dropping CJS being breaking? I forget; we've been on the 2.0.0-beta branch for so long (many years), until very recently catching up to rc. I do know at some point I'd tweaked our fork to transpile Quill TS into CJS rather than ESM, but after catching up to upstream, I've removed that tweak because it no longer seemed needed.

[luin]

@alecgibson Yeah I tried a couple of approaches and none of them works well (turns out default exporting is hard). So I think we have to keep the current approach (we export ESM for years in 2.0.0-dev.4 so don't consider it as a breaking change).

For Node.js usage, users would have to transpile the files. Fortunately it's straightforward with Jest:

// jest.config.js
module.exports = {
  transform: {
    "\\.[jt]sx?$": "babel-jest",
  },
  transformIgnorePatterns: ["node_modules/(?!quill|parchment|lodash-es)"],
};

[alecgibson]

@luin you mean you're not changing anything? Feels a bit broken exporting ESM code without "type": "module" set? The above repro is pretty simple, and I find it weird that it's broken. Or would be nice to maybe export both CJS and ESM? I think that's generally better practice.

[fnlctrl]

@alecgibson found a maybe relavent stackoverflow post about using mocha with esmodules https://stackoverflow.com/a/60522428
I think you can add a "type: module" to your own package.json (not quill's) or use babel:

mocha -r @babel/register

[alecgibson]

Adding "type": "module" to your own package.json doesn't fix the issue. We still get compiler issues, because quill is using import statements without declaring itself as ESM:

[...]/node_modules/quill/core.js:1
import Quill from './core/quill';
^^^^^^

SyntaxError: Cannot use import statement outside a module

I don't understand why we can't just set "type": "module" in quill if it's ESM?

[luin]

I'm trying make Quill a pure ESM package and so far it works good: #4047 and it passed https://arethetypeswrong.github.io/

I can't find a way to support both CJS and ESM at the same time due to bundlers behave differently on handling default exports, especially as we are also exposing TypeScript declarations.

[alecgibson]

Okay I think so long as Quill goes properly ESM, that would be good. This weird inbetween state of using ESM but not declaring ESM is poorly defined in my opinion.