fix(nm): declare @yarnpkg/pnp as a dependency

[zkochan]

What's the problem this PR addresses?

Currently when using the nm package directly, there is compilation error because pnp is not in the deps:

How did you fix it?

...

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[larixer]

Stop, guys @zkochan and @RDIL , it doesn't make sense. @yarnpkg/nm doesn't use anything from @yarnpkg/pnp except types. Have you tried to figure out deeper before just adding @yarnpkg/pnp as a prod dependency?

[larixer]

In other words the compiled code of @yarnpkg/nm has no imports from @yarnpkg/pnp at all, it does not makes sense to me, that @yanrpkg/pnp is a production dependency in this case, what do you think?

[arcanis]

Fwiw I personally have no qualms with it being a regular dependency, my thinking being that regular deps are meant to be all the deps required for a package to work properly once installed by users, whether it's at runtime or typecheck.

Anecdotically, it's the same interpretation we follow in @yarnpkg/extensions:

• berry/packages/yarnpkg-extensions/sources/index.ts

  Line 163 in 589e231

  "@babel/types": `^7.8.3`,

• berry/packages/yarnpkg-extensions/sources/index.ts

  Line 878 in 589e231

  '@types/unist': `^2.0.0`,

• berry/packages/yarnpkg-extensions/sources/index.ts

  Lines 56 to 68 in 589e231

  	// https://github.com/sindresorhus/got/pull/1125
  	[`got@<11`, {
  	dependencies: {
  	[`@types/responselike`]: `^1.0.0`,
  	[`@types/keyv`]: `^3.1.1`,
  	},
  	}],
  	// https://github.com/szmarczak/cacheable-lookup/pull/12
  	[`cacheable-lookup@<4.1.2`, {
  	dependencies: {
  	[`@types/keyv`]: `^3.1.1`,
  	},
  	}],

Peer deps also don't seem a right fit to me, because they delegate the responsibility of installing packages to the users, which goes against what Yarn is for (managing packages / versions). The only reason we'd do this would be for what essentially amounts to optional size optimization, and I admit I'm not a fan of this tradeoff.

Ideally it'd be nice to support a typesDependencies field, but that would require cooperation between all package managers to be useful, and perhaps we should wait to see what happens of the TC39 proposal regarding types.

[larixer]

Fwiw I personally have no qualms with it being a regular dependency, my thinking being that regular deps are meant to be all the deps required for a package to work properly once installed by users, whether it's at runtime or typecheck.

What is your objective point here, except your sole opinion?

Anecdotically, it's the same interpretation we follow in @yarnpkg/extensions:

We could have used devDependencies in this case as well, so I don't see how this can be considered as a counterpoint at all.

Let's discuss seriously.

[larixer]

I think in this case the @yarnpkg/pnp should be declared in peerDependencies instead. I cannot find a better specification for package.json fields then from here:
https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependencies
and assume the majority of the people rely on the meanings documented there.

Read the section about peerDependencies and compare with this case. @yarnpkg/nm does not require @yarnpkg/pnp it just expresses compatibility on the interface level, that's why @yarnpkg/pnp should be a peer dependency.

[zkochan]

Compatibility is already guaranteed by the types. Not compatible code will not compile. I have not yet seen any types declared as peer dependencies. It would worsen the DX. Especially as Yarn doesn't automatically install the peer deps.

[larixer]

Compatibility is already guaranteed by the types. Not compatible code will not compile.

Of course not compatible code will not compile, but you don't know with which version of the package it is compatible. That's why peer dependency expresses the version of the package it is compatible with.

I have not yet seen any types declared as peer dependencies.

Can't be a point in discussion.

It would worsen the DX. Especially as Yarn doesn't automatically install the peer deps.

It depends on the tools you use and how you use them. Having dependencies declared as production that are really not production can worsen your DX too, think of extra install time and extra disk space consumed by an application in the cloud.

[zkochan]

The file that I need doesn't use pnp at all, so if you choose to make it a peer, it would be nice to move the hoist function to a separate package.