install: log failed optional dependencies as info instead of warn (fixes #11632)

[oocx]

As described in issue #11632, npm outputs warning messages if optional dependencies fail to install because they are incompatible with the current operating system. npm outputs warnings even though everything completed successfully as expected. This is confusing and it trains people to ignore warnings, which might cause them to later overlook a "real" warning.

I changed the verbosity for these messages from "warn" to "info".

This is my first pull request for npm. I hope I followed all contribution guidelines. Let me know if this pull request is ok or if I need to make changes.

[othiym23]

@iarna what do you think of this change? Specifically, I'm not sure how I feel about changing the warning label based on the specific conditions included.

[Vanuan]

@othiym23 optionalDependencies is a hack, but there's no other way to depend on different npm packages for different platforms. Is there?

[iarna]

@othiym23 Mmm, yeah, I'm concerned about that aspect too. First of all, this is not the only error that can arise for an optional dependency– any solution needs to be more comprehensive.

I think if we're changing this to info then we shouldn't be using the tree's warnings array at all. That specifically exists to defer printing warnings until after the install tree is printed, to ensure that they're visible to the user. So instead of adding a special case in lib/install.js, I would change andHandleOptionalErrors in lib/install/deps.js to use log.info instead of treeWarn.

The other case of warnings on optional installs is when they fail during lifecycle scripts. That's handled over in lib/install/actions.js in andHandleOptionalDepErrors. That's already just doing a log.warn so changing that is much easier.

In both cases it's not quite as simple as just printing out the error as the error object needs to be passed to lib/utils/error-message.js to get a message suitable for printing. (The latter case doesn't currently do that, but it's an oversight.)

[glen-84]

This should be a debug message ...

#11632 (comment)
#11632 (comment)

[whitneyland]

@iarna hi Becca, I first thought it might not help to reply but I liked your bio, and was encouraged by the fact we both seem to "prefer the pragmatic to the idealistic".

Would you be willing to consider reopening this one in the spirit of such pragmatism? Some reasons are:

• Productivity. The success of npm not withstanding, there are still vast numbers of developers ramping up on the platform. A significant number of which are taking time to worry about and investigate this problem (duplicate issues have more activity than this one). You have the power to single handedly do more for the economy than the new tax bill. Ok that's a joke. But still true given trickle down has already been proven not to work.

• Perfect should not be the enemy of the good. After coming up to speed on the issue I'm aware there may not be an elegant quick fix. But the upside is a less than ideal fix does not have to be permanent. We could do something now that would allow clean builds and a better developer experience for 100k+ people, and when the multiple involved parties do come to agree on a long term solution it can still be implemented.

• Free PRs. Maybe the team would want to do this themselves, maybe not. But the good news is they both options available. There are several people willing to help as needed.

• It actually matters - I don't know if you are much into things like work process optimization theory or the effects of tiny amounts of friction on the conversion rates of e-commerce transactions. I'm not recommending it, it's pretty dry stuff to be honest. The point is that that seemingly small amounts of friction, distraction, etc can be significant. Similar to how some managers now realize that making a dev go to a status meeting for 7 minutes, 8 times a day cannot be accounted for by adding 1 hours to a schedule.

Also, a probably not persuasive quote from a not very good movie but with some lengendary acting, bet you can't guess it:
"Society cannot exist without justice yet it is also built on compassion. Let us set the tone by making a historic gesture. Ladies and gentlemen, I implore you, let us give this npm warning issue a second chance".

[iarna]

This was replaced by #17530 which the author then closed. As I said there, I'd be happy to accept a version of this that directly calls log.info instead of fiddling with the warnings array.

[YasharF]

New PR: #19198