Distribute ESM module in addition to CJS

[feljx]

Would it be possible to distribute an ESM module version in addition to the current CommonJS version?
I know it's not a clean solution (gotta use "main", "module" and "exports" fields in package.json to work with all setups), but more and more libraries are distributing both so as to enable users to switch over to ESM.

[lpinca]

Yes, probably, but I see no upsides, only downsides.

[lpinca]

For example we could use an ES module wrapper as documented here https://nodejs.org/api/packages.html#packages_approach_1_use_an_es_module_wrapper.

$ git diff --cached
diff --git a/package.json b/package.json
index c2d63af..360ac2d 100644
--- a/package.json
+++ b/package.json
@@ -16,6 +16,10 @@
   "author": "Einar Otto Stangvik <einaros@gmail.com> (http://2x.io)",
   "license": "MIT",
   "main": "index.js",
+  "exports": {
+    "import": "./wrapper.mjs",
+    "require": "./index.js"
+  },
   "browser": "browser.js",
   "engines": {
     "node": ">=8.3.0"
@@ -23,7 +27,8 @@
   "files": [
     "browser.js",
     "index.js",
-    "lib/*.js"
+    "lib/*.js",
+    "wrapper.mjs"
   ],
   "scripts": {
     "test": "nyc --reporter=lcov --reporter=text mocha --throw-deprecation test/*.test.js",
diff --git a/wrapper.mjs b/wrapper.mjs
new file mode 100644
index 0000000..8df8956
--- /dev/null
+++ b/wrapper.mjs
@@ -0,0 +1,9 @@
+import createWebSocketStream from './lib/stream.js';
+import Receiver from './lib/receiver.js';
+import Sender from './lib/sender.js';
+import WebSocket from './lib/websocket.js';
+import WebSocketServer from './lib/websocket-server.js';
+
+export { createWebSocketStream, Receiver, Sender, WebSocket, WebSocketServer };
+
+export default WebSocket;

but this might introduce issues with bundlers and would break code that uses the library like this

const createWebSocketStream = require('ws/lib/stream');

At the very least we should bump the major version and I'm not sure if it makes sense when this

import WebSocket from 'ws';

already works without changes.

[jimmywarting]

i vote for ditching commonjs.

but this might introduce issues with bundlers and would break code that uses the library like this

We only target node versions, it's not like it's running in the browser. and uses <script> or <script type="module">
Deno already have WebSocket built in... So it's unlikely this will be used on any other environment...?
ppl don't bundle stuff on the server side

if someone is still using commonjs then they can import it using the async import()

fetch-blob, node-fetch, formdata-polyfill are some of wish have already ditched commonjs for being only ESM packages

[lpinca]

ppl don't bundle stuff on the server side

Unfortunately this is not true :( A lot of people bundle for the server side. If you search through closed issues there are some examples of people bundling for Electron or Lambda functions.

Also, I don't want to drop Node.js 10 support in the next major release and there are some blockers for a pure ESM migration like https://github.com/websockets/ws/blob/7.5.2/lib/validation.js#L85-L104 and https://github.com/websockets/ws/blob/7.5.2/lib/buffer-util.js#L104-L129.

[jimmywarting]

Unfortunately this is not true :( A lot of people bundle for the server side. If you search through closed issues there are some examples of people bundling for Electron or Lambda functions.

hmm.

some blockers for a pure ESM migration like...

could be solved with top-level await?
like i have done in another ESM only package: https://github.com/jimmywarting/native-file-system-adapter/blob/master/src/FileSystemWritableFileStream.js#L4

edit: that did require 14.8... https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await#browser_compatibility :(

[lpinca]

could be solved with top-level await?

It needs to be sync, so no I don't think so.

[lpinca]

It could actually work with top-level await.

[jimmywarting]

It could actually work with top-level await.

I know it can work
in my package i hade a class that needed to be exported and extended on WritableStream (conditionally from polyfill) so i had to load WritableStream async depending on if it's browser or node

This is how i mange to ship one package that don't Bundle everything and conditionally loads from either cdn or npm if needed and not having to bundle many different version for each enviorment

all ESM imports are loaded asynchronous like a asyncGenerator (even if you don't know it) hence why commonjs package can only load ESM packages with import(name) and never with require(name) (b/c of top level await)
it truly dose not feel sync when you do import x from 'foo' but it really is behind the seen

[lpinca]

Yes, but what are the advantages over a simple wrapper like this #1886 (comment) that works everywhere?

[jimmywarting]

Yes, but what are the advantages over a simple wrapper like this #1886 (comment) that works everywhere?

the differences is that you only export it as a ESM package... Like a Wrapper. You are not using ESM yourself anywhere in your own codebase cuz you use require on every other place instead.

• The result: you can't use top-level await or import from x
• the fact that you only use commonjs (require) in your codebase result in you being unable to load other packages that are written with only ESM like "fetch-blob" for instances.
  • it opens up a new world but makes it shitty for other ppl still using commonjs when you yourself only publish a full ESM package
• and also: It would only work with import ws from 'ws' not with import x from 'ws/server.js'

...this year is probably going to be the worst when we are in a transition face from going from "UMD-whatever" to ESM standard
I chose to push for standardization in my own package to speed things up. WebTorrent is also on the path of going all ESM once they solve jest test runner to run with type=module

[lpinca]

• The result: you can't use top-level await or import from x
• the fact that you only use commonjs (require) in your codebase result in you being unable to load other packages that are written with only ESM like "fetch-blob" for instances.

I don't think this is a problem for ws. It only has 2 optional (CJS) peer dependencies that live under the same organization. There are no plans to add more dependencies.

It could only be an issue if some dev dependency decides to go ESM only but even in that case we are not forced to update it.

[jimmywarting]

☝️ That's correct
was thinking the same thing - you would probably not benefit so much from it, if you didn't need top level await then it could be enoghf to support 12.20 but top level await was added in 14,8 so... there is that: don't sound like you are wiling to go the mile and wish to support something like 10 or 12 or something like that

[jimmywarting]

btw, here is a interesting article i found earlier: https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1 if you want to read some tech stuff related to JS/ESM

[lpinca]

Yes, I know it's a mess. I received hundreds of emails from people discussing this endlessly in the Node.js organization. Top-level await would only be needed for the two optional peer dependencies mentioned above (bufferutil and utf-8-validate). Both of them live in the websockets org so there is no risk that they will switch to ESM only in a blink of an eye.

I honestly don't think it's a good idea for ws to migrate to ESM only.

TL;DR we don't need top-level await, not now, not in the near future.

[jimmywarting]

I honestly don't think it's a good idea for ws to migrate to ESM only.

You are probably right, a #1886 (comment) would be fine for now for a transition period.

Glad we could have this level of discussion. (where neither did not butcher the other person thoughts/ideas without hearing out first what both ppl where thinking and discussing pro & conn)

I would wish that you put it in a backlog doe to ditch commonjs entirely once the time is right, like in the "next-next" major release or some milestone. (along with native Event + EventTarget & Blob support?)

---

It would be cool if you partially supported Blob look-a-like items in ws.send(blob) it dose not mean that you have to depend on Blob from v15 and return it in messages when bufferType='blob'

just check if it's a object with a arrayBuffer() and do data = await obj.arrayBuffer()
that way ppl using fetch-blob, node-fetch or node:buffer.Blob can send blobs if they would like too, could probably do some more type checking

[lpinca]

I would wish that you put it in a backlog doe to ditch commonjs entirely once the time is right, like in the "next-next" major release or some milestone.

Yes, probably when Node.js 14 will no longer be supported. Backwards compatibility is nice.

(along with native Event + EventTarget & Blob support?)

The first step here would be exporting NodeEventTarget in Node.js core so that people can use it. Last time I tried it was not exported.

Edit: even if it is exported NodeEventTarget cannot be used as is, too many limitations. We would need to replace the EventEmitter with the EventTarget completely.

[lpinca]

It would be cool if you partially supported Blob look-a-like items in ws.send(blob) it dose not mean that you have to depend on Blob from v15 and return it in messages when bufferType='blob'

just check if it's a object with a arrayBuffer() and do data = await obj.arrayBuffer()
that way ppl using fetch-blob, node-fetch or node:buffer.Blob can send blobs if they would like too, could probably do some more type checking

The ws.send() method was not designed to handle promises and this is basically only API sugar for

const buf = await blob.arrayBuffer();
ws.send(buf);

Users can already do that.

[chase-moskal]

hello, i came here after finding socket.io to fail with false esm support

they used a wrapper.mjs strategy which actually fails, because the wrapper merely re-exports the commonjs, which actually fails in modern browsers

it appears in your latest commit on this issue that ws is making the same mistake

i'm really hoping somebody on earth can make a working es module library for web sockets, so i don't have to ;)

tag me in and i'd be happy to participate in a code review so we can make sure this actually works for ws, thanks

[lpinca]

they used a wrapper.mjs strategy which actually fails, because the wrapper merely re-exports the commonjs, which actually fails in modern browsers.

I don't think this is a problem. ws does not work in the browser.

[chase-moskal]

@lpinca — okay, i understand now — this makes sense

• socket.io's situation is a problem because they're not actually a web socket library, not compatible with native browser websockets, so they need a working clientside library
• whereas ws actually is a real websocket library, and thus, is compatible with the native browser websockets — thus it's totally fine for ws to be a serverside-only library, since using the brower's WebSocket constructor is the preferable method

that clears it up for me, sorry about the false alarm, please proceed!