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

Wrong typings #5116

Open
1 of 4 tasks
Warxcell opened this issue Feb 10, 2023 · 5 comments
Open
1 of 4 tasks

Wrong typings #5116

Warxcell opened this issue Feb 10, 2023 · 5 comments

Comments

@Warxcell
Copy link
Contributor

Warxcell commented Feb 10, 2023

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

Make sure to fork this template and run yarn generate in the terminal.

Please make sure Mesh package versions under package.json matches yours.

  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

on stitching resolver: the return type is wrong

image

it says the return types is the Object returned by the graphql schema, but it says nothing about graphql errors - so you actually miss them, until they blow in your face at runtime as shown with this commit.

image

Furthermore: .then((data) => ...) has a wrong type also, when stitching is batched. ( Warxcell/graphql-mesh-wrong-typing@5790275 )

image

as seen on log:

image

To Reproduce Steps to reproduce the behavior:

https://github.com/Warxcell/graphql-mesh-wrong-typing

Expected behavior

return type to be the type from graphql | Error.

Environment:

not related, versions are seen in the repo.

Additional context

@Warxcell
Copy link
Contributor Author

Also Context doesn't contains req/res objects. 😕

@ardatan
Copy link
Owner

ardatan commented Feb 13, 2023

@Warxcell req and res are Node specific. Mesh's runtime doesn't know what env you are using. You can use the generic request object that contains all the information you need from the incoming request. However you can extend the context type as you wish.
https://the-guild.dev/graphql/mesh/docs/guides/graphql-code-generator#customizing-the-graphql-code-generator-configuration

About returning errors, does then returns Error because they might need to be handled with .catch instead of .try.
About array results, this one is interesting. Probably it is a bug in typings but you should be able to extract individual entities with valuesFromResults.

Also be careful with the in-context SDK because data manipulation should be done very carefully there. If the return types don't match, Mesh will take the selection set of the current resolved field and this might be the thing you want;

type Query {
  bar: Bar
}
type Bar {
  barField: String
}
type Foo {
  fooField: String
}

So

Query: {
  bar: (root, args, context, info) => {
     const data = await context.MYAPI.Query.foo({
       ...
     })
     // data.fooField is undefined here because `Foo` and `Bar` are different types;
     // You have to provide a selection set `{ fooField }` in this case.
   }
}

@Warxcell
Copy link
Contributor Author

Warxcell commented Feb 13, 2023

However you can extend the context type as you wish.
https://the-guild.dev/graphql/mesh/docs/guides/graphql-code-generator#customizing-the-graphql-code-generator-configuration

As fas as I saw: all the context is generated by envelop plugins - in that case - woudn't be best if mesh is started programatically rather than with command that does something behind the scenes? That way context will work out-of-the-box and no need to worry about adding plugins. Right now I have custom context and I put it everywhere as typehint, but that doesn't guarantee from end-2-end that it works.

About returning errors, does then returns Error because they might need to be handled with .catch instead of .try

one could care only about success results? in that case it can just forward the error. with current typings that is not possible either, because GraphQLError is not assignable to return type of then which is the type itself.

image

About array results, this one is interesting. Probably it is a bug in typings but you should be able to extract individual entities with valuesFromResults.

true, but use-case is other: there is a resolver composition, on calling API 1, it modifies the args by calling API 2 - .then is used to modify the args - which is not concern of valuesFromResults, no?

@ardatan
Copy link
Owner

ardatan commented Feb 13, 2023

@Warxcell Mesh CLI allows you to start a server using Node.js in an easier way. If you want to have a custom server configuration, you can use HTTP handler and pass the context however you want. For example in AWS Lambda, we pass the context like that.
https://the-guild.dev/graphql/mesh/docs/getting-started/deploy-mesh-gateway#deploy-mesh-on-aws-lambda
However, request is already available in most cases.
Still you can use Mesh programmatically as explained here.
https://the-guild.dev/graphql/mesh/docs/guides/mesh-sdk
Basically Mesh is not able to know how you execute it. It is user's choice. It can be through an HTTP Server, a local SDK or just execute.

Going back to the error handling, I am a bit confused :) When you call a method from an in-context SDK, it rejects the promise in case of an error or resolves with the data as typed. Resolvers composition might make things tricky here which I need to see in an example probably. Maybe you can create a very minimal isolated sandbox on CodeSandbox or StackBlitz just for that.

@Warxcell
Copy link
Contributor Author

it rejects the promise in case of an error or resolves with the data as typed

Actually no - in case of error - it resolves with the error.

This was referenced Apr 30, 2024
This was referenced May 7, 2024
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

2 participants