install: use log.info for incompatible optional deps

[watilde]

If the target environment of optional dependency is different from user's, it should be skipped. Since it is an expected behaviour, it should not be displayed by default.

Related issues:

• Info, rather than Warn, when skipping packages not suitable to target OS #11632
• Please Re-Open https://github.com/npm/npm/issues/11632 #17500

[glen-84]

It should probably be a DEBUG message:

#11632 (comment)
#11632 (comment)

[oocx]

I created a very similar PR last year, and it was not accepted (#12841). I hope you have more luck with your PR, this bug is really annoying!

[watilde]

@glen-84 That makes sense since the build error for optional deps doesn't mean failed result. I will update the patch to use info instead of notice. Thanks!

@oocx I'm sorry to see that and I did not notice it ;(
I'd love to put your name to co-credit in the commit message eventually.

[oocx]

@waltilde Thanks, that's not necessary. As long as this bug gets fixed, I don't care about who gets the credit :)

[watilde]

I just updated the patch and the log is not displayed by default anymore.

Here is the actual output:

• https://gist.github.com/watilde/ee528c7e4d3685c62fdfdb584ce8e39d

[iarna]

#12841 wasn't rejected, I asked for changes to the PR.

And those requested changes still apply: These shouldn't go through the deferred warnings system.

(We may ultimately ditch the deferred warning system entirely now that installs are terse enough that deferred warnings don't improve readability.)

[oocx]

That's right, I was not very clear in my wording - the simple change that modified deferred warnings was rejected, a more complex / comprehensive solution would probably have been accepted. I did not implement those changes back then as I thought that for me as an outsider, they might require too much time to understand what needs to be changed. Maybe watilde can make those changes. If not, I would probably try to implement the requested changes.

[whitneyland]

So @iarna is @watilde being rejected on this or what? You say @oocx wasn't rejected but practically speaking he was.

It's amazing that the only factor here seems to be allegiance to some kind of perceived elegant design. I've not seen anyone on the team willing to take a shot at weighing any shortcomings of these PRs against the estimated wasted time and productivity on a global scale, which is quantifiably large as a few google searches can show. Engineering is supposed to be science combined with trade offs, compromises, toward maximum utility.

To get this fixed I've tried to offer funding, tried to politely discuss the reasons I feel it's important, neither received any response. I know people are busy - I wouldnt mind bringing lunch to your office to talk about possible solutions. How can we make this happen?

[watilde]

I will continue working on this issue :)

I thought it could be simple design if we can add tree.info, but I realised that it requires change so many flows in the install process after an investigation. Then I will try Rebecca's original plan for next step.

[iarna]

@whitneyland Asking for minor code changes does not constitute rejection. I'm always happy to to help contributors if they ask for it, but no one did. This isn't a matter of elegance, this is a matter of hacking something in sidewase versus something along the lines of log.info('abc', 'def'). Avoiding the deferred warning system with the latter is actually simpler.

@watilde Call npmlog's info method directly, don't store them on the tree. =)

[YasharF]

Replacement PR: #19198