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

Revisit decision to forward previous value #31

Closed
briancavalier opened this issue Apr 25, 2012 · 2 comments
Closed

Revisit decision to forward previous value #31

briancavalier opened this issue Apr 25, 2012 · 2 comments
Assignees
Milestone

Comments

@briancavalier
Copy link
Member

Early on, I made a decision that when a handler returned undefined, the previous promise value should be forwarded through to the next promise. That's convenient in some cases, as it allows reusing and writing handlers that have no return statement.

However, it means that it's easy to get into trouble when doing things like this:

when(somethingHappens, function(result) {
    return result["thePropertyIReallyCareAbout"];
}).then(handleProperty);

In cases like that, where the first handler legitimately needs to return undefined, and we want handleProperty to receive undefined, it will instead receive result, which is just plain wrong. Right now, the workaround is:

when(somethingHappens, function(result) {
    return result["thePropertyIReallyCareAbout"] || null;
}).then(handleProperty);

which is ok, but probably would cause someone a real headache trying to figure it out, and means that handleProperty receives null instead of undefined.

@briancavalier
Copy link
Member Author

An initial version of this is in the dev-20 branch. You can track that to see if this breaks your code. The thing to look for to know if this has broken your code is suddenly seeing undefined being passed into some callbacks or errbacks.

The way to fix it is to find promise callbacks or errbacks where you aren't explicitly returning a value, and return something. In many of those cases, simply returning the callback's input param will fix the problem, but it's best to take a few seconds to evaluate each situation to make sure that's the right thing.

@briancavalier
Copy link
Member Author

Moving this up to 1.3 since it's important for compatibility when assimilating jQuery.get() that returns a rejected $.Deferred.

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

1 participant