Add fastify to peerDependencies

[paulius-valiunas]

Fixes #114

[gurgunday]

Hi!

Why is this needed? Is there any problems without it?

[paulius-valiunas]

Hi!

Why is this needed? Is there any problems without it?

yes, see the linked issue. It simply doesn't work with pnpm.

[gurgunday]

Can you delete it from devDependencies then?

https://github.com/fastify/fastify-type-provider-typebox/pull/165/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R64

I'm not sure about this just because we only use peerDeps as a last resort, let me verify it with others

Cc @mcollina @climba03003

[paulius-valiunas]

Can you delete it from devDependencies then?

#165 (files)

I'm not sure about this just because we only use peerDeps as a last resort, let me verify it with others

Cc @mcollina @climba03003

No, it has to be in both. Everything that you declare in peerDependencies should also be a devDependency, otherwise you won't be able to build your package.

And to make it clear, your package absolutely needs fastify to be installed alongside it, it doesn't make any sense without fastify, so not declaring it as either a dependency or a peerDependency is a bug. If you declare it as a direct dependency, that will probably work but might cause similar errors when the user installs a different version than what you have declared. Honestly, I think this is a textbook example of the kind of problems peerDependencies were designed to solve.

[gurgunday]

And to make it clear, your package absolutely needs fastify to be installed alongside it, it doesn't make any sense without fastify, so not declaring it as either a dependency or a peerDependency is a bug. If you declare it as a direct dependency, that will probably work

This is what I'm saying, we might prefer a direct dependency here

[climba03003]

And to make it clear, your package absolutely needs fastify to be installed alongside it, it doesn't make any sense without fastify, so not declaring it as either a dependency or a peerDependency is a bug. If you declare it as a direct dependency, that will probably work

This is what I'm saying, we might prefer a direct dependency here

As TypeScript perspective, I would avoid a direct dependency only for types.

As a maintainer of fastify, I would avoid using peerDependices as possible. Adding in one packages will spread the request to all others plugin.

[paulius-valiunas]

But please also consider the "but" part 😅 I'm not saying it won't work at all, but it will most probably cause someone problems with incompatible versions being installed. For example, if fastify 5.0 changes the type definition of FastifyTypeProvider and your package is still on 4.x, users will get a confusing error like in #114 instead of an immediate warning when running pnpm install.

[paulius-valiunas]

As a maintainer of fastify, I would avoid using peerDependices as possible. Adding in one packages will spread the request to all others plugin.

@climba03003 In that case, how do you propose pnpm users should consume plugins?

[paulius-valiunas]

@climba03003 Could you elaborate on why you think declaring fastify as a peerDependency (in every plugin) is bad?

Here's my reasoning in favor of peerDependencies:

A fastify plugin is not a standalone package, it extends a fastify instance that's already declared somewhere in your project. It makes no sense for it to bring its own version of fastify. When a user installs a fastify plugin, he does not expect to get a brand new fastify object, potentially from a different version of the package than the one he's using, he just wants the plugin to extend what he already has installed. If you declare fastify as a direct dependency, you're very likely to get two distinct instances of the package in the dependency tree, which will cause type mismatches, failed instanceof checks etc. It also prevents the package manager from checking if your fastify versions are compatible (i.e. it won't complain if you're using fastify 5.0 but one of your plugins only supports 4.x), which will cause unexpected compatibility problems that can be hard to debug if you don't know to check the dependencies first. Moreover, as this original blog post indicates, "plugins" such as express.js middleware were the reason peerDependencies were introduced in the first place. Fastify plugins fall into the same category, so there's no better way to use peerDependencies than this.

[climba03003]

Could you elaborate on why you think declaring fastify as a peerDependency (in every plugin) is bad?

Not going to debate, but you can see the reason why we do not accept peerDependencies in the history.
https://github.com/search?q=org%3Afastify+peerDependency+&type=issues

[paulius-valiunas]

Could you elaborate on why you think declaring fastify as a peerDependency (in every plugin) is bad?

Not going to debate, but you can see the reason why we do not accept peerDependencies in the history. github.com/search?q=org:fastify+peerDependency+&type=issues

And what exactly is the reason? I see a bunch of unrelated issues mostly caused by incorrectly declared version ranges.

[paulius-valiunas]

is there any hope this could still be merged?

[gurgunday]

I'm personally only concerned with npm, and I think in general we should prioritize npm, but maybe we can make an exception for the type providers

Unfortunately I can't move the ball on this though

[paulius-valiunas]

This will not break npm though.

[markandrus]

The same issue occurs in recent versions of Yarn. For now, the problem can be worked around by adding the following to .yarnrc.yml in a Yarn project's root:

packageExtensions:
  "@fastify/type-provider-typebox@5.x":
    peerDependencies:
      "fastify": "5.x"

[markandrus]

I suppose this can be changed to 5.x now?

Suggested change

	"fastify": "^5.0.0-alpha.3"
	"fastify": "5.x"

[paulius-valiunas]

Thank you, I changed it to standard semver notation.

[paulius-valiunas]

@Fdawgs since I see you're the main contributor to this repo, I would appreciate your opinion on this, so that we could either merge or close the PR.

[jsumners]

The conversation in fastify/fastify-mongodb#209 still stands.

[paulius-valiunas]

We should not support or facilitate using old and buggy software versions that should not include breaking changes.

You are doing exactly that by not having a peerDependency. If you have a peerDependency, you can specify the version range you want people to use. If you don't... well, npm will make it work with any ancient version because it doesn't do any checks, and pnpm will just not work at all.

[paulius-valiunas]

@mcollina I see you previously approved and merged #78. Perhaps you could reconsider this one too?