stop using optional deps to fix webpack

[BryanCrotaz]

ecc-jsbn and tweetnacl appear to not be optional

[StefanFeederle]

My webpack build also fails because it's missing tweetnacl, bcrypt-pbkdf, jsbn and ecc-jsbn.

[Immortalin]

Any luck getting this to run on browser?

[factoidforrest]

These packages being misclassified is causing compatibility issues with certain build systems. This is an issue.

[factoidforrest]

Easy fix. Opened a PR #55

[arekinath]

Do you have a specific error message that might be helpful to figure out what's going on that causes it not to work? The deps ecc-jsbn, tweetnacl etc that are in optionalDependencies are there because sshpk very carefully only calls require() on them if you use the actual functions that depend on those modules (i.e. you can require('sshpk') and it does not require any of those modules automatically -- you have to do something like make an ED25519 key and call .sign() on it before we require('tweetnacl')).

If webpack is not able to deal with the require() calls being optional and inside functions, I suggest that would be a bug in webpack, not in this module. But if there's somewhere where we are accidentally causing these modules to be require()'d when they aren't actually been used, I would like to know about that so I can fix it.

Sorry this has been sitting open for so long -- browser issues are not really a high priority for me (or the other people at Joyent who have worked on this module), and I can't really offer support for how to use sshpk with webpack in general (since I don't use it), but I'm happy to take reports of something more specific, like a require() happening when it shouldn't and get them fixed up.

[factoidforrest]

@arekinath Well, with webpack, even if they MIGHT get required in some function...they still need to be included. If the code ends up in the browser, and then the function gets called, the browser needs to have the package already loaded. It's just how webpack works, and for good reason. Webpack doesn't know and can't know if you're going to call that function or not. All it knows is that you might, so the dependency needs to be bundled. Does that make sense? It's not a bug in webpack. It absolutely needs to work that way, and can't work any other way.

At first glance, the way you are using optional dependencies makes sense. Unfortunately I think that is not the convention. Optional dependencies are almost always used for something that is more performant on a given system, but the module will run fine without them. They are definitely not intended for functionality that might not get called.

I think the very simple fix is simply to move them into dependencies. Almost everyone has been getting them anyway since optional dependencies are almost always installed. This is just breaking certain uncommon build systems that expect people to use the flag properly. Please head into the rush issue I referenced above for an example

[factoidforrest]

@arekinath @BryanCrotaz @Immortalin @StefanFeederle

[arekinath]

Merged a change in the latest release based on @light24bulbs PR which moves all these into regular dependencies, and also changes the require() calls to just happen at startup (at the top of the files). Thanks!