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

Added the readyStateChange event to the blacklist #15

Merged
merged 1 commit into from
Jan 11, 2014

Conversation

lpinca
Copy link
Collaborator

@lpinca lpinca commented Jan 11, 2014

The title says pretty much everything :)

cayasso added a commit that referenced this pull request Jan 11, 2014
Added the `readyStateChange` event to the blacklist
@cayasso cayasso merged commit 31be3a8 into cayasso:master Jan 11, 2014
@cayasso
Copy link
Owner

cayasso commented Jan 11, 2014

Thank you again @lpinca

@lpinca lpinca deleted the fix/reserved-event branch January 11, 2014 19:15
@cayasso
Copy link
Owner

cayasso commented Feb 2, 2014

Working!!! 890463f

@cayasso
Copy link
Owner

cayasso commented Feb 3, 2014

Hey @3rd-Eden I think it would be a good idea to expose the list of reserved events besides the check method, something like bellow, this is helpful for example if you need to black list both client and server events on a single method:

Primus.prototype.reserved = function reserved(evt) {
  return (/^(incoming|outgoing)::/).test(evt)
  || evt in this.reservedEvents;
};

Primus.prototype.reservedEvents = {
  readyStateChange: 1,
  reconnecting: 1,
  reconnect: 1,
  offline: 1,
  timeout: 1,
  online: 1,
  error: 1,
  close: 1,
  open: 1,
  data: 1,
  end: 1
};

@lpinca
Copy link
Collaborator Author

lpinca commented Feb 3, 2014

I've created an issue (#16) for better discussion.
I didn't remember that there were also custom reserved events which were not part of the Primus events.
This way there will be two blacklist, but it is still good to maintain the list of Primus reserved events inside of Primus.

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