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

Reduce package dependencies #725

Closed
p-kuen opened this issue Feb 6, 2022 · 10 comments
Closed

Reduce package dependencies #725

p-kuen opened this issue Feb 6, 2022 · 10 comments

Comments

@p-kuen
Copy link
Contributor

p-kuen commented Feb 6, 2022

I installed mercurius the first time today and I recognized it has a lot of dependencies which could be reduced quite easily in my opinion:

promise.allsettled:

As of NodeJS 12.9 and up Promise.allsettled is available natively without a package dependency. If there would be a node minimal version of 12.9 set, we could remove this dependency (especially because this package has a lot of polyfill dependencies too)

events.on:

As of NodeJS 12.16 and up events.on is available natively without a package dependency. If there would be a node minimal version of 12.16 set, we could remove this dependency

ws and fastify-websocket

It is sad that everyone who does not use the websocket feature has to install these dependencies. In my opinion it would be better if this feature is a plugin for mercurius.

graphql-jit

In my case, I am using code-first graphql (no gql language used). Therefore I don't need graphql-jit, yet I have the dependency installed and even get a peer dependency error because I am using graphql >= 16. I would recommend using graphql-jit as a plugin.

readable-stream

I do not exactly know as of which version of NodeJS readableStreams are supported, but I am sure, most of the users of mercurius do not need this polyfill.

undici

In the code I identified undici to be used for gateways. It would be perfect, if this was a plugin for mercurius too as I think most users do not use this feature.

I would be glad to help reducing dependencies of this package. :)

@mcollina
Copy link
Collaborator

mcollina commented Feb 6, 2022

If you would like to send PRs for promise.allsettled and events.on, it would be amazing.

As for graphql-jit, it seems it is not clear to you how it works. It will improve the performance of your queries independently on how you generate the schema. The peerDep warning does not affect the actual operations and it will soon be fixed. It's great to keep it where it is.

readable-stream is not removable as it will be installed by other dependencies too.

As for the gateway and websockets, they are not that easy to remove and it would severely complicate the maintenance of this module. Extracting them as plugins is possible - but we would have to shift this repo to a monorepo as they are fundamental part of this module. If you'd like the challenge, I'll be happy to review a PR.

p-kuen added a commit to p-kuen/mercurius that referenced this issue Apr 16, 2022
…tion

BREAKING CHANGE: drop support for node <12.9.0

Refs: mercurius-js#725
@simoneb
Copy link
Collaborator

simoneb commented Jul 11, 2022

@p-kuen any progress on this?

@p-kuen
Copy link
Contributor Author

p-kuen commented Jul 11, 2022

My PR for promise.allsettled was already merged and released, events.on would be my next try. Will do that soon!

@marco-ippolito
Copy link
Contributor

I'll remove events.on

@marco-ippolito
Copy link
Contributor

@mcollina are there any other packages from this issue that could be worth trying to remove?

@mcollina
Copy link
Collaborator

mcollina commented Dec 1, 2022

I think extracting federation & gateway will reduce the size significantly.

@p-kuen
Copy link
Contributor Author

p-kuen commented Dec 2, 2022

This would be amazing but quite a huge challenge. @mcollina You thought about a monorepo, right? Would it be possible to create a separate repo as a plugin for mercurius?

@mcollina
Copy link
Collaborator

mcollina commented Dec 2, 2022

Here is it: #916.

@codeflyer is on it.

@simoneb
Copy link
Collaborator

simoneb commented Dec 2, 2022

If we have removed all the external deps that we wanted to remove, and since the gateway is already in a different repo, shall we consider closing this issue then?

@mcollina mcollina closed this as completed Dec 2, 2022
@mcollina
Copy link
Collaborator

mcollina commented Dec 2, 2022

yes

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

No branches or pull requests

4 participants