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

Refactor ws4redis.js #287

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Refactor ws4redis.js #287

wants to merge 2 commits into from

Conversation

aitanadev
Copy link

Refactor to ES6

  • As ES6 Class
  • eslint standard
  • without jquery
  • new callbacks: error, reconnecting, heartbeatTimeout
  • new options: maxAttempts, maxMissedHeartbeats, heartbeatLapse, autoConnect

Refactor to ES6
 + As ES6 Class
 + eslint standard
 + without jquery
 + new callbacks: error, reconnecting, heartbeatTimeout
 + new options: maxAttempts, maxMissedHeartbeats, heartbeatLapse, autoConnect
@DanBrink91
Copy link

this is great, removing the need to include jquery (or anything else like it) in the project. 👍

@jrief
Copy link
Owner

jrief commented Nov 8, 2019

Even though untested, this is a great pull request!
However, how do you see the problem of not supporting ES5 anymore?
Wouldn't it be rather better to offer both version and/or offering a precompiled one.
I'm unsure how many browsers support websocket, but do not offer ES6 support. Are there any numbers?

@aitanadev
Copy link
Author

Thank you!

websockets/ES6 support is the same 96%

https://caniuse.com/?search=es6

https://caniuse.com/?search=websockets

Partially incomplete IE11 support does not affect this commit, so full browser support is identical.

imagen
imagen

Perhaps it is possible to keep the current version as ws4redis.es5.js?

@aitanadev
Copy link
Author

Something like:

  • ws4redis.js (ES6, Eslint standard, standalone)
  • ws4redis.es5.js (ES5, jquery)

Additionally, I can make another independent PR for the ws4redis.es5.js version, eliminating the need for jquery only.

Export default for standar ES6 modules import
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.

3 participants