Feedback

[paf31]

Sorry I missed your IRC message.

I like this idea. Here is some feedback:

• I would move the tests into a tests directory
• Give context a foreign type, or even use Data.Foreign.Foreign.
• I would use Eff, since window is mutable.

[Fresheyeball]

Interesting. Ok, so I agree on Eff, but I'm not sure what you are looking for with Data.Foreign.Foreign.

[paf31]

Well it is unsafe to use the type forall e. { | e} because then I can access any property, even those which don't exist.

forall e. { | e} is an antipattern in my opinion. I don't know where it came from, but a lot of libraries seem to use it to represent "any record". Given parametricity, that type should be uninhabited.

[Fresheyeball]

So, yes its unsafe. But doesn't IsForeign require that all the properties be defined upfront? This needs to be extensible. So an extensible record was the tool I reached for.

[paf31]

You don't need to use IsForeign if you don't want to. For example:

vendor = context >>= getVendor
  where
  getVendor ctx = do
    nav <- ctx ! "navigator"
    vdr <- nav ! "vendor"
    readString vdr

My point is just that it is unsafe, so I think it should be marked that way, either by name or by type.

[Fresheyeball]

I agree with your point. What is the | in your example? What is the type of context in your example? context :: Eff (stuff) Foreign? Its still not clear to me how to make this flexible and safe.

[paf31]

! is defined in Data.Foreign.Index.

It really depends how safe you want to me. Another option is to just give it the type forall eff a. Eff (context :: Context | eff) a, but to rename it to something like unsafeContext, to make it clear that the user needs to watch out for the types.

Ideally in that case, the user wouldn't use unsafeContext directly, but would alias it with a stricter type:

window :: Eff ... { navigator :: { vendor :: String } }
window = unsafeContext

A third and better option (in my opinion) is to use a foreign type:

foreign import data Context :: *

The user can then choose to coerce it if they want to.

[Fresheyeball]

Ok I had this foreign import data Context :: * because its expressive.

purescript-context/src/Context.purs

Line 3 in 3531306

foreign import data Context :: *

But I got rid of it because I couldn't get it to unify for a record type. How would it be coerced?