Discord.js: npm shrinkwrap fix

[0-vortex]

What is the current behaviour ?

Installing discord.js from NPM repository works but trying to use npm shrinkwrap you are required to install packages mentioned in peerDependencies and they are locked down in your tree.
The result is a series of committed or non-committed packages added to your installation tree if you use npm shrinkwrap, a more tedious and error-prone process than installing discord.js with all peers and semantically equivalent.

What is the expected behaviour ?

You should be able to install discord.js as normal without the extra packages required for voice communication, sockets, etc. Immediately trying to shrinkwrap would still fail and point you to install the discord.js-extra (naming suggestions wanted) which contains the extra packages required in optionalDependencies.

A user requiring to shrinkwrap would have a dedicated paragraph in the README where the NPM documented default behaviour is given an example:

npm install -S discord.js node-opus
npm install discord.js-extra --no-optional
npm shrinkwrap

A user not requiring to shrinkwrap would follow current standard procedures since nobody wants breaking changes !

What changes does this issue involve ?

For a successful npm shrinkwrap to work with a random number of extra packages (as required) I could identify the following changes needed:

• removing peerDependencies content in favour of discord.js-extra
• creating and maintaining a discord.js-extra NPM package with current peerDependencies listed as optionalDependencies
• amending the README with information on how to successfully run npm shrinkwrap

Please check this out @Gawdl3y @hydrabolt

[CyberiumShadow]

You're better off using the yarn package manager

[0-vortex]

This is an NPM package with a basic feature completely broken. One requiring to use this package in the NODE ecosystem or without being limited by YARN's license would have to switch technologies.
This is a BUG. I've opened this issue in the scope of contributing a resolution that would benefit more people, a workaround had already been discussed in the previous PR.

[devsnek]

fork d.js and make discord.js-but-with-shrinkwrap-compat then.

[0-vortex]

Yes @GusCaplan that is the only solution right now and also one I implemented before in the pull request that you were part of as well. This issue, as noted, was specifically created to architect a better solution than the one in #1215 which wasn't coherent to the library's core values.

I don't like to opinionate and the hard facts for me right now are:

• you can't fully use this framework with NPM as your package manager
• overall documentation for this framework is minimalistic
• while API level documentation is pristine none of the quirks of this framework have been documented

I appreciate the support contributors provide in your discord server - that is giving up a lot of your free time to resolve various undocumented issues and quirks, however, royalty support is not an excuse to dismiss issues and shun contributions.

[Gawdl3y]

NPM has been, currently is, and at this rate, always will be garbage in some way/shape/form. There are various aspects of it that are fundamentally broken, and show no signs of improving any time in the near future. I have no interest in working around its deficiencies, especially when it complicates things that little bit more, for both us (the lib devs) and our users. Using peer dependencies is already "wrong", and is simply a band-aid fix for NPM's optional dependency support being absolute trash.
"Fixing" shrinkwrap by moving the peer dependencies to a separate package really does very little to help anything. It potentially makes the life of the majority of the user base slightly more difficult, in order to support a feature of NPM that isn't even commonly used. And best of all, you still won't be able to use shrinkwrap if you want to use any of the optional packages (unless you manually ensure it is within the correct version range, and don't install the theoretical discord.js-extra package).

And to respond to other things you mention:

• You absolutely can use this framework with NPM, as 90% (or more) of our user base is doing. The others are using yarn, the slightly less terrible manager. Believe it or not, you are the first person ever to mention npm shrinkwrap that I've seen.
• I'm very well aware of the importance of documentation, and I've always put large emphasis on it since I joined the project. The documentation is always being improved. I personally have been working on rewriting and adding more detail to significant portions of the it, though there is still loads more to be done. If you find areas that are lacking, you are free to submit a PR, or point it out to us.
• I have no idea what you mean by "quirks" of the framework.
• Your issue is not being dismissed, and your contribution is not being shunned. We appreciate that you want to improve the library. We simply disagree about the necessity of this change, and believe that the costs outweigh the benefits.

[0-vortex]

You dismiss my points based on opinions again, this is why I say you shun.
Shrinkwrapping is not a wow process but it's been there for way longer than another custom package manager. It's reliable in its purpose which is primarily a security concern and time saver. Locking down specific versions of your dependencies in an automated release cycle is a must so having to use something other than NPM for it is a quirk that should be at least noted somewhere.

Regarding hard facts, just because I am the first to talk about shrinkwrap it doesn't prove that other people did not experience issues or take decisions based on their capacity to use that feature and unless you have metrics to properly assess this usage then it is based on current user experience and mediums of exposure (github+discord). Keeping current users happy is just one many of the core values you and the other contributors share - I criticised this because having them written down somewhere, can speed up (even if minimalistic increase) the whole thought process of a person that is maybe not using discord even casually.

In reality this is an opinionated limitation, sensible or not it does change the way you expect users to handle this framework. Now don't get me wrong when I first started using djs I was aware of a very well written tutorial (that had better documentation of some of the quirks here). I thought that book was great but the author's rude lack of professionalism put me off and he also insulted everyone that starred his efforts by taking it down permanently - that is too much documentation, everything I meant here was in the form of a few extra lines (~4), gists really that would fast-forward some of the simple stuff like deployment and security. Leaving API level documentation which most people generate automatically the documentation here consists of 50-100 lines. I have personally witnessed at least 4 cases where one of the core contributors provided more support than the entirety of this framework for more trivial questions than shrinkwrap and if you consider this to be large emphasis on the docs present on github I can understand why shrinkwrap is such a minor issue.

If no resolution is consistent to current needs or even acceptable I would still need to be pointed out a way to note this limitation down (where), circular discussions about it are not beneficial to anyone involved.

[devsnek]

the author of that tutorial (who is a she by the way) transferred the tutorial in question here

[meew0]

(The above link is wrong, it should be https://yorkaargh.gitbooks.io/discord-js-bot-guide/)

[devsnek]

rip uri scheme

[0-vortex]

I have given this a lot of thought and I have decided to write this as a last attempt to raise awareness to the importance and benefit of having this fixed properly.

This is not a thorough assessment of all the possible fixes but my own opinionated list based on what I find to be sensible:

• npm shrinkwrap => not working, someone would have to fork d.js and keep it up to date with every release (true peerDependency mechanic) - this is what I am running right now
• locking binary packages => hard to keep up to date with changes
• yarn => only thing that works

Now, before everyone starts picking on me for not using yarn on this, while I am (maybe) the only documented case you have encountered to use shrinkwrap, using a 3rd party package manager is a security concern.
In this case it is not a choice, it is the only solution to a problem that you people just fail to admit - it's not NPM's fault you use peerDependencies this way, it's your choice. It's my choice to use this framework, just as any other NPM package, but wait, hold up, I can't really use NPM with this, not without giving up on a basic (default) feature, changing the way I deploy and possibly wasting A LOT of time in the process.

Does it not make sense to you that some people just don't want to go through this process ? Even niche percentage of your user base.

Let me remind you of some facts:

• if you ever forget to add the proper flags to NPM CLI commands you can always cancel the command
• the optional packages you are so worried about get installed because you don't use shinkrwrap or config
• I have previously tried to fix this issue the way it's documented in https://docs.npmjs.com/misc/config

Call me rude but I just have a hunch nobody involved in this thought of just doing npm config set optional false which is kind of a big deal to me because I linked the documentation before just in case anyone needed to refresh version 4 changes.
I don't think it's ethical to blatantly disregard standard usage without at least letting your users know what the limitations of using this software are.

peerDependencies -> optionalDependencies

and

npm config set optional false

Really worth a 20 day talk to have this misuse fixed for everyone ?

Best wishes