We should have a better way to handle this optimal dependencies

[daviddias]

Seeing this message alerts me that something is wrong.

WARNING in ./~/libp2p-crypto/src/keys/index.js
Module not found: Error: Can't resolve 'libp2p-crypto-secp256k1' in '/Users/koruza/code/js-libp2p-secio/node_modules/libp2p-crypto/src/keys'
 @ ./~/libp2p-crypto/src/keys/index.js 9:29-63
 @ ./~/libp2p-crypto/src/index.js
 @ ./src/support.js
 @ ./src/handshake/propose.js
 @ ./src/handshake/index.js
 @ ./src/index.js
 @ multi ./src/index.js

How can we improve this?

[alanshaw]

I currently can't browserify ipfs-api because of this...

[dignifiedquire]

@alanshaw you can pass --ignore-missing to browserify and it should build fine

[daviddias]

Oh! This is even worse than I thought then, we can't be ok to break people's build process this way.

If we want to enable custom crypto cyphers in order to promote devs to manage their bundle size, we can:

• Make libp2p-crypto a singleton and add a feature to 'addSupport'
  • This has the problem that as soon as there is two modules that depend on two different versions, npm will create two singletons and then we will have issues of it to fail in some parts of the code
• DI all the way down
• Revert this change
• Just add support by default, it is true that it adds to the bundle size, but we want to have ECDSA for record signing anyway.

I'm leaning towards the last two options, being that the last is the one more time efficient and that fits more uses.

[dignifiedquire]

Those solutions don't sound better to me :( I don't understand why browserify is not able to understand optionalDependencies, this seems like a bug to me, given that it works fine in node.

[victorb]

Webpack seems get along with this fine, outputting a warning but continuing anyways so maybe bug in browserify...

[dignifiedquire]

This might be an option: https://github.com/devongovett/browserify-optional

[dignifiedquire]

We are also missing the optionalDependency listing of the module currently.

[dignifiedquire]

We are also missing the optionalDependency listing of the module currently.

nvm, we shouldn't add it because then npm always installs it

[dignifiedquire]

@alanshaw @diasdavid I've created #72

[daviddias]

Ok, now it is all about getting this documented - #71

@alanshaw My apologies for the troubles :(