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

#2404 Modal open calls should complete in order #4302

Closed
wants to merge 1 commit into from

Conversation

nlwillia
Copy link

This is a second PR attempt for issue #2404. (PR #2443 has rebase issues and will be closed.)

@wesleycho
Copy link
Contributor

On superficial review, this LGTM, but I'd like to review it more once we fix the tests in master and test this out.

@wesleycho wesleycho added this to the 0.13.4 (Performance) milestone Aug 28, 2015
@wesleycho wesleycho closed this in 1bba8b4 Aug 28, 2015
@nlwillia
Copy link
Author

I wanted to comment on your last remark on #2443 questioning the decision to have a chained modal resolve even if a modal above it in the stack was rejected. It's a fair point. If you're firing off a stack of modals and one in the middle fails, then continuing to open other modals on top of where it would have been is questionable. There isn't really a recovery strategy there since you can't insert into the middle of the stack.

On the other hand, exposing the rejected value from higher in the stack to modals below is a minor encapsulation leak. Those modals may not be aware that they are stacked, and receiving a rejection from outside their own scope is questionable. And the stacked modals could (due to the asynchronous nature of resolution) be fully resolved and ready to go as far as their dependencies go which might create issues in the client code if it's not aware that a successful resolve could be blocked by an outside factor.

I left the code as it was (ignoring an upstream rejection) for now. Going the other way isn't hard, but I don't think there's a completely satisfactory approach, and it doesn't seem likely to be a common concern.

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

Successfully merging this pull request may close these issues.

2 participants