haskell / mtl

The Monad Transformer Library
http://www.haskell.org/haskellwiki/Monad_Transformers
Other
362 stars 63 forks source link

MTL doesn't adhere to PVP as it re-exports e.g. Control.Monad #68

Closed phadej closed 2 years ago

phadej commented 5 years ago

This is subtle issue. But when you do

import Prelude ()
import Prelude.Compat --base-compat-0.11

import Control.Monad.Reader -- mtl-2.2.2

Then the fail is ambiguous.

it's my own fault, that I imported Control.Monad.Reader unqualified, but still this is surprising and nasty.

chessai commented 4 years ago

would it be okay to just remove this?

phadej commented 4 years ago

Remove re-export? I'd be very fine with that, the added convenience is minuscule IMO.

ekmett commented 3 years ago

I think we should just pull off the bandaid and do this.

phadej commented 2 years ago

As the #99 is reverted this should be re-opened.

ekmett commented 2 years ago

So much for pulling off the bandaid. sigh

ekmett commented 2 years ago

I do want to go squarely on record as saying that reverting this change is the wrong decision.

sjakobi commented 2 years ago

@ekmett Are you aware of the discussion in https://github.com/haskell/mtl/issues/101 about how to best respond to the breakage that pulling off this bandaid would cause?

The decision to revert it certainly wasn't done lightly.

ekmett commented 2 years ago

Yes. That level of impact is well within my expectation.

This has been a wart in the design of this library that we've suffered with for 15 years, because at no point has it ever been "the right time to fix it." The thing is, waiting to fix it ever longer will simply increases our exposure without end.

If our stated intention is to do this in 3.0 then all that we do by pushing this off is make sure that the substantive changes we want to make there to improve are conflated with this and ensuring we have even more code to unbreak.

If our intention is not to do this in 3.0 then I really don't think this is maintenance, it is life support.

ekmett commented 2 years ago

The patches consist of adding imports, not making complicated semantic changes.

The cost of the status quo is that adding any new combinators to Control.Monad or Control.Monad.Fix or Control.Monad.Trans means that any existing user of MTL modules that uses any of the new names we take in base may have to hide the same import a dozen times or more to keep using their code.

With perfect hindsight, pulling this bandaid off when it was causing this pain to users more actively during the MonadFail switch-over would have been better.

ekmett commented 2 years ago

Why is it rational that import Control.Monad.RWS should bring into scope fix?

Meanwhile, import Control.Monad.RWS.Class spuriously does not.

chessai commented 2 years ago

(fixed a typo)

See https://github.com/haskell/mtl/pull/103#issuecomment-1024522023

Bodigrim commented 2 years ago

This has been a wart in the design of this library that we've suffered with for 15 years, because at no point has it ever been "the right time to fix it." The thing is, waiting to fix it ever longer will simply increases our exposure without end.

I do not see many developers suffering from this wart for 15 years. If that was the case, we'd see lots of packages choosing to import mtl qualified or with explicit import list, but this is not so.

The breakage allows for backwards-compatible patches, which proponents of the change could have provided beforehand to alleviate the pain of migration. Yet I've seen no such effort underway in 2.5 years this ticket is open.

I believe such a nuclear change, breaking all downstream dependencies, deserves a public presentation and community discussion before being made. A brief line in a changelog does not make it justice.

ekmett commented 2 years ago

There has been no effort underway on anything around the mtl in the 2.5 years this ticket has been open.

emilypi commented 2 years ago

with mtl-2.3, i believe we addressed this. Please hmu to reopen if there's more non-compliance.