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

A lightweight fix for webofthings/webofthings.js#3. #7

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

Conversation

franckOL
Copy link

Another to try to fix #3. (ES6 compatibility)
It works for me, it's cleaner that last time

@domguinard
Copy link
Member

Hi Franck, looking forward to testing this (probably this weekend), I also worked on a fix for that last weekend but used Watch.js so your solution looks more elegant. Thanks again!

@domguinard
Copy link
Member

That's the branch with Watch.js btw: https://github.com/webofthings/webofthings.js/tree/observe

@domguinard
Copy link
Member

Hi Frank,

I did test it and it works very nicely on Node 6 but the WS subscription does not work anymore on Node 4. Note entirely sure why yet, did you check the tests on Node 4 (npm test)? Did it work for you?

@franckOL
Copy link
Author

Hi Dominique,
I not test on Node 4. I see if I can test today. I don't run npm test if fact.

@franckOL
Copy link
Author

Hello,

I made somechange. npm test works for Node 4 and node 6. Hope that's OK for 7

@domguinard
Copy link
Member

It looks great, thank! I had just seen that indeed somehow it always fell back to the custom defined ArrayObserver, not sure I understand why yet. Will test the code on the target devices (Pi) asap and will get back to you but this is looking promising!

@domguinard
Copy link
Member

domguinard commented Dec 11, 2016

Hi Franck,

I did test it on the hardware (Pi) and there is an issue: when a client listens for Websockets notifications about Actions (e.g., on/off), the actual Action handler (in corePlugin.js or its implementor, e.g., ledsPlugin.js) function is not called anymore. From what I can see you did add support for multiple callbacks in websockets.js but not in corePlugin.js.
I'll try to reproduce what you were doing in websockets.js and will see if that works.

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.

Object.observe is obsolete with ES6. Try to find a solution with Proxy
2 participants