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

.notify & .subscribe stoped working after 0.4.4 upgrade #24

Closed
hoffmannjan opened this issue Jul 14, 2017 · 10 comments
Closed

.notify & .subscribe stoped working after 0.4.4 upgrade #24

hoffmannjan opened this issue Jul 14, 2017 · 10 comments

Comments

@hoffmannjan
Copy link

hoffmannjan commented Jul 14, 2017

charREAD.on('read', function(data, err) {
	console.log('>>> charREAD notify', data, err);
});

charREAD.subscribe()

This code simply stopped working when upgrading from 0.3.2 to 0.4.4 :(.

@hoffmannjan hoffmannjan changed the title .notify() & .subscribe stoped working after 0.4.4 upgrade .notify & .subscribe stoped working after 0.4.4 upgrade Jul 14, 2017
@jasongin
Copy link
Owner

Are you sure? None of the changes since 0.3.2 have affected any code related to notify & subscribe.

@jasongin
Copy link
Owner

Also, the test I use all the time includes subscribe & notify, and it still works fine.

@hoffmannjan
Copy link
Author

100% sure, tested this again and have the same problem.
I was also astonished looking in git diff of this update, that something like this can stopped working.

@hoffmannjan
Copy link
Author

hoffmannjan commented Jul 19, 2017

Here's the code that I'm using for this test
https://pastebin.com/X9xeiRrR

@jasongin
Copy link
Owner

That code looks straightforward. So this still doesn't make sense to me. I'll try to investigate more when I have time.

@jasongin
Copy link
Owner

Is anyone else having this problem?

@taktran
Copy link

taktran commented Jul 31, 2017

I've tried this with a stripped down app, and can confirm that the .notify's stopped working from 0.3.2 onwards

I've tried

  • 0.4.2 to 0.5.1 - didn't get any callbacks
  • 0.4.1 - didn't compile for me
  • 0.3.2 - worked! Would trigger callbacks on notify

@jasongin
Copy link
Owner

@taktran Thanks for checking all the versions. That narrowed it down, then I found the problem!

One of the places where I had manually patched the generated code got accidentally reverted:
6126c38#diff-f5cdb08420f05042c8348dbd32ee22d6L6603

I originally made that patch because it was crucial to enable notifications. I'm just not sure why notifications continued to work for me after it was reverted, so that I didn't catch that mistake when testing.

Anyway, I'll fix this and publish an update today.

@jasongin
Copy link
Owner

jasongin commented Aug 1, 2017

Fixed in 0.5.2.

@jasongin jasongin closed this as completed Aug 1, 2017
@taktran
Copy link

taktran commented Aug 1, 2017

Have tested on 0.5.2, and notifications work as expected. Thanks a bunch @jasongin ! 👍

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

No branches or pull requests

3 participants