fix: --dry-run with --json output

[greggman]

This broke in 10.4. Before 10.4 npm install --dry-run --json would return JSON. After 10.4 it no longer returns JSON.

https://github.com/npm/cli/commit/c209e989b405fa3e86df7015c22e6840e18313b8#r165769221

Note: This is the smallest change and I didn't update any tests. Nor was I sure where to one. The fact that it broke means there was no test for this before.

If it was up to me I think I'd add a summary.diff = [] and push the diffs on there. And then only at the end where it outputs summary, if you want diff output to stdout then map over summary.diff. That would put the checks for output in a single place.

References

Fixeds #8565

[wraithgar]

Yeah we'll definitely want a regression test for this in test/lib/utils/reify-output.js. There are two tests there already for dry-run and long you can use as a base for this new test.

[greggman]

you assigned this to yourself. does that mean you're planning to fix it? Or, should I put up a PR with tests?

also, so you have any thoughts/comments on moving the "showfix" stuff outside the loop and have it print from the summary object? it could either print summary.added, summary.removed, etc, or it could add a summary.diff array. I only suggested the diff array because it keeps order. If order is not important than printing from existing data at the end seems like a good idea?

[wraithgar]

you assigned this to yourself

This is a signal to the rest of the team that I'm reviewing the PR.

[wraithgar]

also, so you have any thoughts/comments on moving the "showfix" stuff outside the loop

I think this is a good iteration and it should be a separate PR. This quick fix for parsing opts should be easier to land since it's just one missing conditional and a test, leaving the optimization for a different PR.