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

Fixing errors with CommonJS import of ws #108

Closed
wants to merge 4 commits into from

Conversation

kellydunn
Copy link

@kellydunn kellydunn commented Jun 8, 2022

Hello there!

When attempting to get a Hello World of y-websocket working as a client running node v17.8.0, I encountered a few errors regarding missing imports for ws in src/y-websocket.js. This PR includes the following to get a fully functioning Hello World moving from the client's perspective:

  • Importing WebSocket explicitly from ws
  • Updating the package.json to explicitly call out this as an ES module (similar PRs: Set type as ES Module #104)
  • Updating README.md for updated boilerplate

Huly®: YJS-754

@kellydunn kellydunn changed the title Fixing errors with CommonJS import of ws. Fixing errors with CommonJS import of ws Jun 8, 2022
@@ -5,6 +5,7 @@
"main": "./dist/y-websocket.cjs",
"module": "./src/y-websocket.js",
"types": "./dist/src/y-websocket.d.ts",
"type": "module",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we can't make this change because it would break others using this package.

const wsProvider = new WebsocketProvider('ws://localhost:1234', 'my-roomname', doc, { WebSocketPolyfill: require('ws') })
import ws from 'ws';

const wsProvider = new WebsocketProvider('ws://localhost:1234', 'my-roomname', doc, { WebSocketPolyfill: ws })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really depends on your bundler whether you need to use the commons version require or the esm version import.

@dmonad dmonad closed this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants