commercialhaskell / path

Typed filepath
122 stars 45 forks source link

aeson dependency #136

Closed 2mol closed 2 months ago

2mol commented 6 years ago

Hi everybody! First of all, this library is really great, and I very much enjoy using it.

I wanted to discuss the dependency on aeson. As far as I can see, the only weight it's pulling, is a toJSON instance in Path.Internal. Since aeson itself has a pretty massive dependency graph, that's a lot of dependency for not a whole lot of feature. I mean, aeson is super nice, but I don't use it in every program.

Let me know if there was already a discussion about this, or if there are some obvious good reasons like avoiding orphan instances that counterbalance my desire for faster initial compilation times. The last thing I want to do is nitpick something good. But personally, I would be really happy if my dependency graph could be trimmed of aeson it's dependencies.

NorfairKing commented 6 years ago

@2mol There are a few options:

It would be worth discussion these points

2mol commented 6 years ago

I looked at how many transitive dependencies aeson would gain by depending on path, and the direct ones are just two: filepath and exceptions. Zero would have been nicer, but I asked people if they would be open to take that tradeoff: https://github.com/bos/aeson/issues/677

Edit: yeah, we can cross off that option.

chshersh commented 5 years ago

@NorfairKing @2mol I also would like to make aeson dependency optional. I would like to use path package in my libraries but I don't want to pull a lot of irrelevant stuff and make dependencies footprint big if this is not necessary because I care about having lightweight libraries. I think that the only solution is to create separate package called path-aeson because you can't specify flags in build-depends in the .cabal file which means that libraries will depend on aeson in any case and you need to disable flag manually every time on use site. So flag is opt-out and path-aeson is opt-in.

This whole orphan instances situation is pretty said. On one hand I want to have light packages that contain only data types (like Tagged, Path, Validation, etc.) and on the other hand I want to have packages that contain only typeclasses (like Profunctor, Comonad, Witherable). I'm actually starting to think that having packages for every cross-combination of data-typeclass is not that crazy because it's the only solution currently that allows to control dependencies (until we will have nice functional programming features like HOF and ADT on package-level...).

chrisdone commented 5 years ago

If it helps to add some direction to this discussion, I think we should avoid conditional compilation where possible. A package is easiest to understand, test and tool when it is just a package, and not N packages depending on configuration.

I can understand not wanting to depend on aeson for some smaller tools. So that leaves only a path-aeson instances package.

How this affects library users: they need to add path-aeson to their .cabal file. That's a pretty easy step.

However, how do we inform end-users when they eventually see No instance for FromJSON (Path ..) that this error is fine and to simply add this dependency? Bite the bullet?

chshersh commented 5 years ago

@chrisdone Thanks for your feedback on this problem! I want to add from my side that things improved a bit since then. Now there's an even better solution than a separate library. Since cabal-3.0 you can use Multiple Public Libraries features to have many independent libraries inside single package. The only example and tutorial of this feature I'm aware of can be found here:

However, how do we inform end-users when they eventually see No instance for FromJSON (Path ..) that this error is fine and to simply add this dependency?

In our projects, we usually specify Migration guide in the CHANGELOG. You can find an example here:

I think this is fine because changing dependency is a separate explicit action. It would be nice to use custom type errors here but this doesn't look possible :disappointed:

NorfairKing commented 2 months ago

Removing a dependency at this point would break backward compatibility hard.