Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove redundant peerDeps #142

Merged
merged 1 commit into from
Sep 21, 2024
Merged

Conversation

gurgunday
Copy link
Member

Peer Dependencies use case:

There's one use case where this falls down, however: plugins. A plugin package is meant to be used with another "host" package, even though it does not always directly use the host package.

We always directly use toad-scheduler and we don't let the user pass a toad-scheduler instance, so I think having it as peerDep is unnecessary here:

function plugin (fastify, opts, next) {
const scheduler = new ToadScheduler()
fastify.decorate('scheduler', scheduler)
fastify.addHook('onClose', (fastify, done) => {
scheduler.stop()
done()
})
next()
}


I can also change the range to ^3 but that might be breaking, not sure

@Fdawgs Fdawgs merged commit ec2cc06 into fastify:main Sep 21, 2024
11 checks passed
@gurgunday gurgunday deleted the remove-peerDeps branch September 21, 2024 08:02
@kibertoad
Copy link
Member

@gurgunday Key reason here is to allow user to bring their own major toad-scheduler version if they choose to do so, without needing to release a new version of fastify-schedule

What are the benefits of a new approach?

@gurgunday
Copy link
Member Author

gurgunday commented Sep 21, 2024

I thought a custom instance would be a setting that’s passed or something, like it is in rate-limit

Also the range only seems to allow versions 2 or 3 and not 4 or higher, so it’s only those two?

@gurgunday
Copy link
Member Author

Yeah I see now, I misunderstood this, I was trying to align it with the custom redis instance handling of rate-limit

In rate-limit, it's not a peerDep and just a passed option, but here it seems like the peerDep is the only option

I'll revert to not make it breaking

@kibertoad
Copy link
Member

@gurgunday let's remove the upper version ceiling, though

@jsumners
Copy link
Member

@gurgunday Key reason here is to allow user to bring their own major toad-scheduler version if they choose to do so, without needing to release a new version of fastify-schedule

What are the benefits of a new approach?

This is why modules like our Redis one provide a way to supply your own instance.

@kibertoad
Copy link
Member

@jsumners but that is an overkill if all I want is a different version

@jsumners
Copy link
Member

I disagree. It is flexible and avoids the nonsense associated with peer dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants