Missing peer dependency on fastify

[andreialecu]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

latest

Plugin version

No response

Node.js version

x

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

x

Description

There's no peer dependency on fastify, which can break things with stricter package managers like Yarn or pnpm.

Steps to Reproduce

See package.json

Expected Behavior

There should be a peer dependency on fastify. I could open a PR to add it myself, but not sure what the version range should be. Is "*" acceptable?

[mcollina]

This is not needded.

peerdependencies are essentially a broken feature across the industry and we stay away from them.

[andreialecu]

Disagree, but ok. :)

For future readers bumping into this. A dependency of some sort is needed for TypeScript because of the types here:

https://github.com/fastify/fastify-secure-session/blob/bb23dc685871d9b998064b77af62ef2178f3a973/types/index.d.ts#L3-L15

Here's the error you might bump into:

src/app.module.ts:67:30 - error TS2339: Property 'session' does not exist on type 'FastifyRequest<RouteGenericInterface, Server, IncomingMessage, FastifySchema, FastifyTypeProviderDefault, unknown, FastifyBaseLogger, ResolveFastifyRequestType<...>>'.

67             session: request.session,
                                ~~~~~~~

That's because fastify being augmented by those types is not guaranteed to be the same fastify your app uses.

https://yarnpkg.com/advanced/rulebook/#packages-should-only-ever-require-what-they-formally-list-in-their-dependencies https://pnpm.io/how-peers-are-resolved

This blocks this package from being usable with Yarn 2+ and pnpm.

A workaround for Yarn 2 is to add this to .yarnrc.yml

packageExtensions:
  "@fastify/secure-session@*":
    peerDependencies:
      fastify: "*"

They cannot be a simple dependency because it will desync with whatever fastify version will be used in the top-level project unless the package manager dedupes (which is also a bad idea for many reasons, but npm does it automatically), so it has to be a peer.

[gino]

I think I am running into this issue as we speak. The error @andreialecu has mentioned, is the exact same issue I am running into after a simple pnpm install. All the code works and this error came out of nowhere:

[gino]

Adding the following section to my package.json fixed this issue for me:

"pnpm": {
  "packageExtensions": {
    "@fastify/secure-session": {
      "peerDependencies": {
        "fastify": "*"
      }
    }
  }
}

[andreialecu]

An option would be adding a note in the README that only npm will work out of the box. The README should at least point out users of yarn and pnpm to this thread or mention the workarounds directly. Alternatively, say that only npm is supported and everyone else is on their own.

The correct fix would be adding the peer dependency, which carries the whole meaning of the relationship between the packages.

As for

peerdependencies are essentially a broken feature across the industry and we stay away from them.

If by the industry it's meant just npm, I'd agree. :)

[andreialecu]

@mcollina just fyi, if this RFC gets implemented in npm, as it has already been accepted, it will introduce a mode similar to pnpm and this issue will pop up there too: https://github.com/npm/rfcs/blob/main/accepted/0042-isolated-mode.md

[mcollina]

What makes this issue specific to this module? Every fastify module is implemented in the same way, and possibly there are some specific on what your setup is that cause this problem with types.

If this problem is not limited to this module, I recommend you to open an issue on the main repo with a complete reproduction, and we'll likely figure out a workaround that does not require the use of peerDependencies.

[andreialecu]

I'm working around it so it's not a blocker by any means. I just wanted to bring more awareness to this issue.

I'm also not sure why the hate on peerDependencies - the problems stemming from them derive from bad decisions npm made, such as auto-installing them.

They serve a very important purpose, especially in mono repos and with workspaces. It's impossible to avoid them in such cases.