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

Remove hawk dependency #2831

Closed
hueniverse opened this issue Dec 15, 2017 · 10 comments
Closed

Remove hawk dependency #2831

hueniverse opened this issue Dec 15, 2017 · 10 comments

Comments

@hueniverse
Copy link
Contributor

hueniverse commented Dec 15, 2017

I can't take this any more.

Request has a dependency on hawk even though no one uses that feature. It was added because @mikeal was using it a few years ago. People who use hawk are more likely to be using other hapi ecosystem modules like wreck and no hawk user needs it integrated like this. I wrote hawk and am its main user and I don't want it integrated in wreck, the module I use instead of request.

Every other week some request user opens an issue on hawk, hoek, boom, or the other dependencies with shitty attitude and a sense of entitlement (even if they think they are being nice about it) demanding changes, asking to support older versions of node, removing dependencies that annoy their build system, complain about how it's not working well with yarn, and other issues I don't care about.

NONE of these people USE hawk, or give a crap about the way I do things. I am not asking them to. These are developers that have not chosen to work within my ecosystem and my way of doing things. This was forced on them as much as on me.

It is also absurd that every month, hawk is downloaded 24 million times, hoek 26 millions times, boom 36 million times, cryptiles 24 million times, and sntp 24 million times FOR NO REASON! That's over 100 millions monthly download of code NO ONE uses.

I am always happy to try and solve real issues people have with my code. I take pride of my work and go out of my way to support people who honored me with building their product using it. But this is not part of the deal. You don't get to waste my time if you don't use my work in a meaningful way.

Please find the less disruptive way to remove this or make it optional in some way. For example, joi has a dependency on isemail that is structured in a way that the module is only loaded when needed which allows people to not require they don't want it.

At some point, I am going to take one-sided actions that will force this move. I am not going to break anything but I sure can annoy the fuck out of everyone with colorful console logs...

We are like a couple who finds out a year later that when got drunk one night and got married. Time for a quick annulment - we are already seeing other people.

@evansnicholas
Copy link

I use the hawk support request includes and am quite happy with it. If it was removed I could of course add the necessary header myself but this does make it easier.

@simov
Copy link
Member

simov commented Dec 15, 2017

@evansnicholas the problem stems from the hardcoded dependencies in request. Removing the hawk dependency and trying to load it only when the hawk option is used will break every browserify build. Which is another funny thing to do, but that's what people do anyway.

@travi
Copy link

travi commented Dec 15, 2017

i also use the hawk capability built into request because of the convenience. i wouldnt be upset if i needed to adjust my usage, but i do find it helpful

@forty
Copy link

forty commented Dec 19, 2017

Couldn't those exotic signature libraries (hawk, http-signature, aws4,...) be optional dependencies? As in "it's required only if you use the feature"? Isn't that what peer dependencies are for? (I came here to see if the crazy number of dependency request has was being discussed and I found this issue at the top :))

@nchilada
Copy link

nchilada commented Jan 3, 2018

+1. hawk and boom appear to have dropped support for older versions of Node during the course of major version releases; on the other hand, request has indirectly dropped support during the course of a minor release.

More concretely, we have a Node app that has broken due to an indirect (and therefore difficult-to-replace) dependency on request ^2. It would be really helpful if #2831 were resolved in a future 2.x.y release.

@mikeal
Copy link
Member

mikeal commented Jan 3, 2018

-1 on changes "because people are sending me PR's I don't like in the dependency." We all get these PR's, me more than most, close them and move on.

-1 on changes "in order to support Node.js v4 and before."

-1 on "optional dependencies" because this features just doesn't actually work well and feels semi-abandoned by npm. I think we even tried using this in the past and pulled it.

-1 on "peer dependencies" because they are even worse than optional deps.

That said, we have way too many deps, and the way features are laid out is problematic and messy which is why so many things are just included and enabled by default.

I'd love to see a more modular way to enable features that would reduce our deps. It's a breaking change and we'd have to push a new major, but if we're going to drop hawk (which isn't even the least used feature/dep in request) we should clean up the rest so that we don't end up doing several major releases.

In the meantime, we'll lock the dep if Eran starts putting terrible console output in hawk releases :P

@mikeal
Copy link
Member

mikeal commented Jan 3, 2018

Removing the hawk dependency and trying to load it only when the hawk option is used will break every browserify build. Which is another funny thing to do, but that's what people do anyway.

Ya, this kind of dynamic import just doesn't work. We need an API change where people actively enable these features by passing in a module that has the necessary deps. That gets the deps out of request and doesn't break browserify and other bundlers. This would also dramatically reduce the default bundle size.

@blacksun1
Copy link

This is now causing an issue due to the old Hawk using an old version of Hoek which is failing an NSP check.

#2874

@JSteunou
Copy link

Adding transform / interceptors / hooks (you name it) to request might ease the removal of hawk (and other not so much used features)

@hueniverse
Copy link
Contributor Author

I think #2943 is a reasonable compromise. Thoughts?

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

9 participants