Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aeson dependency #136

Open
2mol opened this issue Oct 18, 2018 · 5 comments
Open

aeson dependency #136

2mol opened this issue Oct 18, 2018 · 5 comments

Comments

@2mol
Copy link

2mol commented Oct 18, 2018

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
Copy link
Collaborator

@2mol There are a few options:

  • The current situation: path with an aeson dependency
  • Make aeson depend on path and give it a non-orphan instance. (I would be happy with this, but not sure if you'll be able to convince the aeson maintainers. You'll have to open an issue there and refer back if you want to try this.)
  • Make two seperate packages: path and path-aeson where the latter has the instances which would be orphan instances.
  • Make the aeson dependency flag-activated so you can turn it off for your project. This would mess with a bunch of tooling if it's off by default, but we can leave it on by default too.

It would be worth discussion these points

@2mol
Copy link
Author

2mol commented Oct 19, 2018

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: haskell/aeson#677

Edit: yeah, we can cross off that option.

@chshersh
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Contributor

@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 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants