Skip to content

Changed browser to es modules #20

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 6, 2022
Merged

Changed browser to es modules #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b24337
Changed browser to es modules
guillemcordoba Jul 9, 2021
c8943bd
Added rollup example
guillemcordoba Jul 9, 2021
4fae52f
Tested webpack
guillemcordoba Jul 9, 2021
df2045d
Removed module
guillemcordoba Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ if (typeof WebSocket !== 'undefined') {
ws = self.WebSocket || self.MozWebSocket
}

module.exports = ws
export default ws
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like a breaking change, the major version number should be changed when we publish it🤔

4 changes: 4 additions & 0 deletions example/rollup/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import WebSocket from 'isomorphic-ws'


const ws = new WebSocket('ws://www.host.com/path')
19 changes: 19 additions & 0 deletions example/rollup/app.output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// https://github.com/maxogden/websocket-stream/blob/48dc3ddf943e5ada668c31ccd94e9186f02fafbd/ws-fallback.js

var ws = null;

if (typeof WebSocket !== 'undefined') {
ws = WebSocket;
} else if (typeof MozWebSocket !== 'undefined') {
ws = MozWebSocket;
} else if (typeof global !== 'undefined') {
ws = global.WebSocket || global.MozWebSocket;
Comment on lines +9 to +10
Copy link

@JoCat JoCat Sep 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Moreover, checking for global is useless here, because this is a node.js object
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis#description

Historically, accessing the global object has required different syntax in different JavaScript environments. On the web you can use window, self, or frames - but in Web Workers only self will work. In Node.js none of these work, and you must instead use global.

} else if (typeof window !== 'undefined') {
ws = window.WebSocket || window.MozWebSocket;
} else if (typeof self !== 'undefined') {
ws = self.WebSocket || self.MozWebSocket;
}

Copy link
Owner

Choose a reason for hiding this comment

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

should we throw an error when no WebSocket detected, like ws did: https://github.com/websockets/ws/blob/master/browser.js

Copy link

Choose a reason for hiding this comment

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

can better use "globalThis"?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis
especially since most modern browsers work from a regular WebSocket
https://caniuse.com/mdn-api_websocket

var WebSocket$1 = ws;

new WebSocket$1('ws://www.host.com/path');
Loading