search

purescript / purescript-prelude

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

6.0.1 contains a breaking change on `showRecordFields` ? #303

Open jmatsushita opened 1 year ago

jmatsushita commented 1 year ago

PR #299 seems to contain a breaking change CC @ajnsit

For instance it breaks this code:

requiredFields :: Array String
requiredFields = Data.Show.showRecordFields requiredProxy (required record')

and returns this error:

[1/1 TypesDoNotUnify] .spago/option/main/src/Option.purs:440:22

  440      requiredFields = Data.Show.showRecordFields requiredProxy (required record')
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  Could not match type

    String

  with type

    Array String
garyb commented 1 year ago

You're absolutely right, I considered ShowRecordFields to be an internal typeclass since it's a helper for the standard Show instance, forgetting that it has to be exported also, making it essentially public.

I'm not sure what the best course of action is now - to revert this, or to just fix purescript-option since it hasn't been reported anywhere else. I have to assume purescript-option is not in the registry as otherwise this would have become apparent sooner. 🤔

JordanMartinez commented 1 year ago

I think I agree that ShowRecordFields is an internal type class. I think purescript-option should have defined its own type class to handle its particular use case, but maybe they didn't because they would have just reimplemented ShowRecordFields... :thinking: