Cli-microsoft365: Consider adding `npm dedupe` to project upgrade instructions

[waldekmastykarz]

When upgrading SPFx projects from one version to another, when you need to upgrade one or more npm packages, often you end up with a bunch of TS errors about duplicate incompatible types. I found out, that running npm dedupe after upgrading packages, solves this issue and prevents you from having to delete node_modules and the lock file. Should we consider adding npm dedupe to the set of upgrade instructions whenever we suggested upgrading one or more packages?

/cc: @baywet @wobba @VelinGeorgiev

[wobba]

Sounds like a good plan to me! If this solves me not having to nuke the modules folder at times I'm all for it :)

[baywet]

I've not run into that issue so far and I've upgraded a few projects over the last few weeks. It might be related to the fact I'm using node 10 (with npm 6.4), which is not officially supported by SPFX but I found works much better with installations, package-lock creation and a few other things like that.
As far as I know npm install was already supposed to do some level of deduplication https://docs.npmjs.com/cli/install#algorithm
Now with all that being said I don't think it can hurt to add an additional step (which should be documented and optional) saying "if you run into errors like this, run that".

[waldekmastykarz]

If we did it as an optional step, how would that be handled with the shortened command that you've added recently @baywet?

[baywet]

the regex I built only matches npm un and npm i so npm dedupe should not be considered for shortening. The only thing to adapt would be this https://github.com/pnp/office365-cli/blob/7e1dfe5d96bfd09951e859b9efa9eae1f522853a/src/o365/spfx/commands/project/project-upgrade.ts#L347 as far as I can see

[waldekmastykarz]

I could add it as optional and see how it gets surfaced in the report

[sympmarc]

I'm not sure if this is connected or not, but after I run each npm command, I get messages similar to this:

+ @microsoft/[email protected]
updated 1 package and audited 876700 packages in 65.081s
found 5 vulnerabilities (1 low, 4 high)
  run `npm audit fix` to fix them, or `npm audit` for details

The number of vulnerabilities seems consistent. Is this just par for the course (my guess)?

[waldekmastykarz]

What you're seeing @sympmarc is something different. Audited dependencies are not related to duplicate packages and the reported vulnerabilities depend on what's currently tracked in the vulnerabilities database used by npm.

[sympmarc]

That was my guess. I'm wondering if there's a logical place in the docs to explain the fact that the the vulnerability messages may show up and what to think about them? It's tempting to try to clean them up with npm audit fix, but that's likely to cause other issues.

[waldekmastykarz]

Ideally, that would belong in the SPFx docs as it's something that's been coming up, and will be coming up in each SPFx version no matter if you're creating a new project or upgrading an existing one.