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

switch repo fork for the websocket package #2976

Merged
merged 5 commits into from
Jul 31, 2019

Conversation

michaelsbradleyjr
Copy link
Contributor

@michaelsbradleyjr michaelsbradleyjr commented Jul 28, 2019

The michaelsbradleyjr/Websocket-Node#polyfill/globalThis branch uses a more robust approach to resolve a browser's window object. It also includes all the latest commits on the upstream's master branch, including Node v12.x compatibility and a workaround for the .git/ subdir npm bug.

The "preinstall" script in packages/web3-providers-ws/package.json is removed because it's not helpful and its use of rm causes Windows incompatibility. Closes #2971 and #2973.

packages/web3-providers-ws/src/index.js is refactored since the websocket package already does native WebSocket detection.


At some point it will be preferable to install websocket directly from the registry, i.e. by specifying a version number/range in packages/web3-providers-ws/package.json but this is a way to move forward until a future release of the websocket package.

@coveralls
Copy link

coveralls commented Jul 28, 2019

Coverage Status

Coverage increased (+0.04%) to 83.191% when pulling a78399a on michaelsbradleyjr:fix/websocket into fe269fb on ethereum:1.x.

@nivida nivida added 1.x 1.0 related issues Bug Addressing a bug In Progress Currently being worked on labels Jul 29, 2019
Copy link
Contributor

@nivida nivida left a comment

Choose a reason for hiding this comment

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

@nivida nivida requested a review from gnidan July 29, 2019 09:50
The `michaelsbradleyjr/Websocket-Node#polyfill/globalThis` branch uses a more
robust approach to resolve a browser's `window` object. It also includes all
the latest commits on the upstream's `master` branch, including Node v12.x
compatibility and a workaround for the `.git/` subdir npm bug.

The `"preinstall"` script in `packages/web3-providers-ws/package.json` is
removed because it's not helpful and its use of `rm` causes Windows
incompatibility. Closes web3#2971 and web3#2973.

`packages/web3-providers-ws/src/index.js` is refactored since the `websocket`
package already does native WebSocket detection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants