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

dataCallback, shouldSendCallback are passed prior callback as 2nd arg #636

Merged
merged 1 commit into from
Jul 1, 2016

Conversation

benvinegar
Copy link
Contributor

@benvinegar benvinegar commented Jun 30, 2016

Fixes #428 (and eventually #477)

Now you can do the following for both setDataCallback and setShouldSendCallback:

Raven.setDataCallback(function (data, priorDataCallback) {
  if (priorDataCallback) {
    data = priorDataCallback(data);
  }
  data.message = 'derp';
  return data;
});

This way it is possible to preserve OR override the behavior of any previously set callback, without adding new API methods (e.g. addDataCallback and addShouldSendCallback). It also lets subsequent callbacks mutate data before or after the prior callback is invoked, which the add API is not capable of doing (execution would be serial FIFO).

I'm not 100% on doing this, but I think it could be a satisfactory solution – especially since we've gone this far without it. Would love to get the community's feedback.

cc @mattrobenolt @nevir @mitsuhiko

@nevir
Copy link
Contributor

nevir commented Jul 1, 2016

This way it is possible to preserve OR override the behavior of any previously set callback, without adding new API methods (e.g. addDataCallback and addShouldSendCallback).

Yay! Much better

I'm not 100% on doing this, but I think it could be a satisfactory solution – especially since we've gone this far without it. Would love to get the community's feedback.

It would definitely cover my needs for #626

@mattrobenolt
Copy link
Contributor

🍪

@mitsuhiko
Copy link
Member

🍪

@benvinegar benvinegar merged commit cb86145 into master Jul 1, 2016
@benvinegar benvinegar deleted the get-exist-callbacks branch July 1, 2016 22:18
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.

4 participants