Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

How to properly use "update" notification #110

Open
cerupcat opened this issue Oct 29, 2014 · 3 comments
Open

How to properly use "update" notification #110

cerupcat opened this issue Oct 29, 2014 · 3 comments

Comments

@cerupcat
Copy link
Contributor

In meteor, sometimes "update" is sent before "result" and sometimes after.

Currently, if "result" is sent prior to "update", the methodId is removed from _methodIds and therefore, the update notification is never sent because there's no more reference to that methodId.

If "update" is sent before "result", the update notification is sent, but the methodId isn't removed until "result" is sent.

Should we add two methodId sets? One for update and one for result? One updateMethodIds would remove the id after the update message and the resultMethodIds would remove the id after the result message.

@mikey0000
Copy link
Collaborator

Hey @cerupcat thanks for bringing this up, I will try to take a look at this issue and decide on steps forward. Couple of details can you provide and check, does it still behave that way with v1? If so do you have a way to reproduce that behaviour?

@cerupcat
Copy link
Contributor Author

Looking back, it looks like I'm the one who actually added the "update" message code. It wasn't there before our pull request. At that time, we weren't actually using it and had put it in for the future (and to complete the DDP specification). We have a need for the update message now, so that's where I saw the implementation I did doesn't work correctly.

In our own fork, I made a made a change to have both resultMethodIds and updateMethodIds as separate sets. This seems to be working, I'm just not sure if it's the best implementation or not. I can pull request later, if we think that'll work.

@mikey0000
Copy link
Collaborator

Ok yes, if you're using it daily and it's working, make the pr and I can always take a look over it.

On October 31, 2014 11:31:43 AM GMT+13:00, Seth [email protected] wrote:

Looking back, it looks like I'm the one who actually added the "update"
message code. It wasn't there before our pull request. At that time, we
weren't actually using it and had put it in for the future (and to
complete the DDP specification). We have a need for the update message
now, so that's where I saw the implementation I did doesn't work
correctly.

In our own fork, I made a made a change to have both resultMethodIds
and updateMethodIds as separate sets. This seems to be working, I'm
just not sure if it's the best implementation or not. I can pull
request later, if we think that'll work.


Reply to this email directly or view it on GitHub:
#110 (comment)

Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants