search

paf31 / purescript-jquery

Type declarations for jQuery
MIT License
30 stars 28 forks source link

Avoid duplicate labels in rows #15

Closed paf31 closed 10 years ago

paf31 commented 10 years ago

@garyb Could you please review?

paf31 commented 10 years ago

I'll merge this since I think it's pretty safe.

garyb commented 10 years ago

Oops! Sorry, I had meant to comment on this, although I think I wanted to try something first... changing the type signature to the way it is now, doesn't that mean that handlers must have a DOM effect now? So you couldn't just use a trace or something?

paf31 commented 10 years ago

The problem is that you can't use DOM in the handler with the old approach, with the new duplicate labels rule. You can still use things like trace in the handler, but you can't use a closed row with only Trace for example.

Until we get row constraints (I don't know when ...) I think this is the only good option sadly.

paf31 commented 10 years ago

I think this would be a good thing to document in "FFI Tips". I'll add a note.

garyb commented 10 years ago

That's fine, I think having DOM as a required effect for on is hardly ever going to be a problem anyway, aside from debugging with trace or something - I doubt there are many situations where an event handler is not going to do something else to the DOM or at least make use of a DOM-effected-action.

paf31 commented 10 years ago

Well, remember DOM being there doesn't force you to use the DOM, it just means you can.

garyb commented 10 years ago

Ah, ok, that's what I thought it would have meant.

Actually, why did this case trigger the error, if they're both dom :: DOM, shouldn't that unify still? I thought the likes of dom1 :: DOM, dom2 :: DOM would be rejected, but this would be ok.

paf31 commented 10 years ago

No, unification is based on the label, not the type. But if the labels are the same, the types have to unify, of course.

It triggers the error because eff contains dom, so the RHS contains two copies of dom.