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

graphql is not declared as peer dependency #3724

Closed
3 tasks done
andreisergiu98 opened this issue Dec 22, 2024 · 2 comments · May be fixed by #3730
Closed
3 tasks done

graphql is not declared as peer dependency #3724

andreisergiu98 opened this issue Dec 22, 2024 · 2 comments · May be fixed by #3730

Comments

@andreisergiu98
Copy link

Describe the bug

graphql is not declared as a peer dependency neither for the urql package nor @urql/core. While it happens to work with npm and pnpm because of the fs lookup, it breaks for yarn pnp.

While so far, for me, the only thing that broke was the type inference because all graphql types will fallback to any, there is runtime code also which won't work at all.

Solution:
@urql/core and all packages that rely on it must have graphql as required peer dependency.

Reproduction

.

Urql version

v4.2.1

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@kitten
Copy link
Member

kitten commented Dec 23, 2024

graphql isn't a direct or indirect dependency of urql, but instead @urql/core relies on @0no-co/graphql.web which is a replacement that's leaner.
https://github.com/0no-co/graphql.web

That in turn does have types that either rely on graphql or not, depending on whether a code base uses it elsewhere to guarantee that types match up, but has a corresponding optional peer dependency.

If you have a specific question or issue, it might instead be a usage question, for which a discussion thread would be more suited ✌️

For now, since there's no reproduction or more details I'll temporarily close this though

@kitten kitten closed this as not planned Won't fix, can't repro, duplicate, stale Dec 23, 2024
@andreisergiu98
Copy link
Author

andreisergiu98 commented Jan 3, 2025

Hi @kitten,

Thank you for the prompt response! So I did a little more digging to understand 0no-co/graphql.web and urql and I was able to find the root cause.

So if my understanding is correct graphql.web will use types from graphql if it can be resolved, otherwise will fallback to its local definitions.

It does that using a utility type similar to this one:

type Or<T, U> = 0 extends 1 & T ? U : T;

urql-core tries to do the same thing with GraphQLError, DocumentNode and DefinitionNode in packages/core/src/utils/graphql.ts. As a sidenote the last two might be redundant with the last version of graphql.web.

On to the issue at hand. Typescript changed the behaviour of "unresolved" any from 5.5 so doing type Doc = Or<UnresolvedType> | LocalDoc will always be any. Here is the reference to the issue, and it's not considered a defect microsoft/TypeScript#58960

This can be fixed by updating the utility type in both urql and graphql.web to:

type OrFixed<T, U> = void extends T ? U : T;

I will open a PR with a fix for both libraries tomorrow. urql might also need to bump the dependency version of graphql.web.

Here is a reproduction repo https://github.com/andreisergiu98/urql-repro-3724. The affected code resides in client/src/component.tsx. You will require corepack for yarn berry.

With this new information are you willing to reopen the issue?

Also would you consider adding graphql as an optional dependency to urql to satisfy the passthrough for graphql.web, otherwise yarn berry will never resolve graphql for it even if installed?

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 a pull request may close this issue.

2 participants