ethul / purescript-angular

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

Use purescript-simple-dom for #4 #16

Closed dylex closed 9 years ago

dylex commented 9 years ago

This removes the entire DOM module and replaces it with equivalents in purescript-simple-dom. Most of the changes were in Http to use Data.DOM.Simple.Ajax, which now contains a lot of the code that was here. Exported types remain almost identical, with the exception of the MozChunked types (which don't actually exist -- they're just represented by the same underlying types in the response, but with different timing semantics). This addresses #4.

ethul commented 9 years ago

This looks great! Thanks for making these changes.

Feel free to merge, I will leave it up to you if you want to add the commit hash or tag for the dependency. I think it might be a good idea, but we haven't tagged this repo yet, so either way works for me.

Also, I don't know if you have an example worked up using some of the HTTP code, but if so, would you consider adding it to the examples dir? If not, no worries, just if you had something already.

dylex commented 9 years ago

It sounded like simple-dom may get an actual bower release at some point soon, so I thought to wait until then (or at least a tagged version) to fix the dependency.

I started working on an example, but got a bit confused with the promises. If you have an Eff e (Promise ...) (like HttpResponse), you can convert the Promise to a PromiseEff but it seems like you still need unsafeRunPromiseEff to actually do anything. I was expecting a function with type PromiseEff e e a b -> Eff e (Promise a b) but couldn't find one. (I had some other type confusions, like why then' can change the failure type, or why pureResolve exists at all over using Q, but I probably just have to wrap my head around it first.)

ethul commented 9 years ago

Sounds good. We can wait for purescript-simple-dom to publish a tag.

Regarding the promises, I think this area can definitely still use work. As a note on pureResolve, I think we need this in order to write the pure applicative instance for Promise. I don't see another way to do this, but I am open to other ideas here.

Good catch on then', I've opened issue #17 for this so I remember to fix it. Thanks! Also, we should be able to add the function PromiseEff e e a b -> Eff e (Promise a b) as you suggest. In terms of working with Eff and Promise, I would be open to discussing different approaches here. We had a thread going on issue #12, so I'd be up to revive that and continue brainstorming.

Let me know if you've come across anything else. It would be good to review and potentially make improvements.

dylex commented 9 years ago

Thanks. I'll probably fix (and file) more issues as I get more comfortable with it all. Just don't want to rush ahead too rashly.