ethul / purescript-angular

AngularJS 1.2 bindings for PureScript (currently in the experimental stage)
MIT License
23 stars 3 forks source link

Option to eliminate pureResolve from Bifunctor Promise #20

Closed dylex closed 9 years ago

dylex commented 9 years ago

For discussion, may be better approaches.

Picking up on a discussion on #16, pureResolve and pureReject unfortunately create new angular modules just to create a promise. I believe this may especially be an issue in angular 1.3, where the promise framework has been changed to keep more state, so promises from different modules won't play well together (just based on a simple code review, pending is shared state in processQueue).

This is one way to eliminate them from Bifunctor and a couple other places. I'm not sure how to fix Applicative.pure, yet. This approach works because angular looks at the return values from resolve/reject functions and, if they are not promises, treats them as the new result value (which is also far more efficient than constructing another promise). However, there is a problem here in that if the user actually returns a Promise in a thenPure argument, the types will no longer be correct, because angular will have flattened the extra Promise without us knowing (and we can't write the type "anything but Promise"). (The converse is already an issue: it's actually impossible in angular to create promises that resolve to a promise, but the types here don't know that.)

ethul commented 9 years ago

Thanks. I think this looks good to merge.

However, does angular.injector(['ng']).get('$q') in pureResolve actually create a new module? I believe this creates a new injector, loading the modules passed in.

In any case, I am for reducing the usage for pureResolve and pureReject. Also, I'd like to try these out on angular 1.3 before concluding they are problematic --not 100% convinced they are yet, but I may just need to spend more time investigating.

ethul commented 9 years ago

Actually, thinking about this more, I am not sure using promises originating from different modules would be problematic. For instance, if you load in someone's angular library, you should be able to use their API, even if it is a promise-based API. But I can test out pureResolve on 1.3 to be sure.

dylex commented 9 years ago

You're right -- injector doesn't do what I thought it did, there -- I was assuming it was like module, so this may be a non-issue. I still think there would be an issue if you mixed promises from different $rootScope, because it won't trigger the right digest cycle (through $evalAsync).

This came up because in the HTTP example I'm working on, I just had it put the text of the response into the scope to display on the page, but it didn't actually get updated on the page until the next event, which suggested the final effect evaluation was happening outside an $eval. This may be unrelated to pureResolve though, I'll have to keep poking.

dylex commented 9 years ago

One simple way we could get rid of pureResolve completely would be to add another type on top of Promise:

foreign import data NgPromise :: * -> * -> *
data Promise a b = Defer (NgPromise a b) | Resolve b | Reject a

Then pure = Resolve, and there just has to be some special handling in then to unwrap and return/reject.

ethul commented 9 years ago

Closing due to #23