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

Add Typescript typings #14

Open
kdembler opened this issue Aug 10, 2020 · 6 comments
Open

Add Typescript typings #14

kdembler opened this issue Aug 10, 2020 · 6 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed nice to have Non-essential features that are lower priority but still nice to have

Comments

@kdembler
Copy link
Contributor

As the title says, it'd be great to have typings for this library. Seeing Typescript on the rise and in use in many projects (including one I'm working on), it'd make developer experience better.

@jneurock
Copy link
Collaborator

Thanks for the idea! Can you share some pain points where adding TypeScript would have helped you with this library?

@kdembler
Copy link
Contributor Author

It's not that painful since the functionality offered by the library isn't that complex and there's only a handful of functions but it's still nice to have. With typings:

  • when working with TS, a developer wouldn't need to provide module declaration, i.e. declare module '@miragejs/graphql'
  • automatic imports by IDE would be supported
  • developers would get better IntelliSense experience
  • TS would check for correct function call arguments

Like I've said, these aren't very outstanding issues, but it's pretty standard nowadays to offer TS support

@jneurock jneurock added enhancement New feature or request help wanted Extra attention is needed nice to have Non-essential features that are lower priority but still nice to have good first issue Good for newcomers labels Aug 10, 2020
@jneurock
Copy link
Collaborator

Checking in now that #23 was merged to see if this issue can be closed or if there's more work to do. Thanks!!

@kdembler
Copy link
Contributor Author

#23 is certainly a good start but I'd say there's still more work to be done to close this issue. #23 provides basic types that tsc can infer on its own, but for many cases that may not be good enough. For example, let's look at the generated typings for createGraphQLHandler:

export function createGraphQLHandler(graphQLSchema: any | string, mirageSchema: any, { context, resolvers, root }?: {
    context: any;
    resolvers: any;
    root: any;
}): graphQLHandler;

While its certainly much better than nothing, the typings could be improved. In general, any any (:wink:) instance is a place for improvement because any pretty much disables any type checking. graphQLSchema excepts either string or an AST schema - types for this exists inside graphql package. Same with resolvers - those could be typed as well.

There's also one issue I see introduced with this PR, when I try to provide custom resolvers to the handler, I now also need to pass context and root because those aren't marked as optional, so this example from the README won't work in TS-enabled environment:

const graphQLHandler = createGraphQLHandler(graphQLSchema, this.schema, {
  resolvers: {
    Mutation: {
      // Coerce the record returned from the default resolver to a Boolean
      deletePerson: (obj, args, context, info) =>
        !!mirageGraphQLFieldResolver(...arguments)
    }
  }
})

@jneurock
Copy link
Collaborator

Thanks for the feedback!

@jneurock
Copy link
Collaborator

It has taken years to find the time to work on a TypeScript version of this library but I finally got around to it. I have released version 0.2.0-alpha.1 to npm. This version is based on the typescript branch. If anyone wants to give it a try, it would be great to see if there are any issues.

There should be not breaking changes when it comes to creating a request handler (the main functionality) but there are some breaking changes to a few functions that I would be surprised to see anyone use.

Thanks in advance for any feedback!

@jneurock jneurock self-assigned this May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed nice to have Non-essential features that are lower priority but still nice to have
Projects
None yet
Development

No branches or pull requests

2 participants