purescript / purescript-prelude

The PureScript Prelude
BSD 3-Clause "New" or "Revised" License
161 stars 88 forks source link

Add nub constraint on Record Show instance #269

Closed JordanMartinez closed 2 years ago

JordanMartinez commented 2 years ago

Description of the change

Fixes #216. I'm not sure if this would be considered breaking or not.


Checklist:

JordanMartinez commented 2 years ago

Looks like this can't be fixed. CI fails with:


[1/1 NoInstanceFound] src/Data/Show.purs:50:22

  50    show record = case showRecordFields (Proxy :: Proxy ls) record of
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  No type class instance was found for

    Data.Show.ShowRecordFields ls3
                               rs4

  while applying a function showRecordFields
    of type ShowRecordFields t0 t1 => t2 t0 -> Record t1 -> Array String
    to argument Proxy
  while inferring the type of showRecordFields Proxy
  in value declaration showRecord

  where ls3 is a rigid type variable
          bound at (line 0, column 0 - line 0, column 0)
        rs4 is a rigid type variable
          bound at (line 0, column 0 - line 0, column 0)
        t1 is an unknown type
        t0 is an unknown type
        t2 is an unknown type

           Src   Lib   All
Warnings   0     0     0  
Errors     1     0     1  
hdgarrood commented 2 years ago

I think saying "this can't be fixed" is a bit premature. It looks to me like you're (correctly) asking for a Nub constraint as well as the an ShowRecordFields instance for rs' in the Show instance context, but in the actual implementation you're still asking for a ShowRecordFields instance for rs, which you now don't have.

JordanMartinez commented 2 years ago

Ah... That explains the type error.

JordanMartinez commented 2 years ago

I added a second Nub constraint, but thinking about this more, couldn't we also wrote Union () rs' rs?

JordanMartinez commented 2 years ago

:ping_pong: @thomashoneyman

thomashoneyman commented 2 years ago

This one's a bit out of my wheelhouse — perhaps @hdgarrood could take another look?

JordanMartinez commented 2 years ago

So...

  1. Should I update the Changelog file to remove the bugfix entry?
  2. Should I mention Harry's comment as description in the breaking change entry?
hdgarrood commented 2 years ago

I think removing the bugfix entry from changelog makes sense, but I'd say my previous comment is more detail than is appropriate for the changelog.