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

directive graphql-rate-limit does not work #1569

Open
gyl89 opened this issue Nov 10, 2023 · 6 comments
Open

directive graphql-rate-limit does not work #1569

gyl89 opened this issue Nov 10, 2023 · 6 comments
Labels
Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue

Comments

@gyl89
Copy link

gyl89 commented Nov 10, 2023

Describe the Bug
@ratelimit directive does not seem to work (coming from the package graphql-rate-limit-directive)

To Reproduce
Define resolverClasses (including one mutation with @ratelimit), then to build the schema, use the code

import {rateLimitDirective} from 'graphql-rate-limit-directive';
  ...
private async buildSchema(resolverClasses: any): Promise<GraphQLSchema> {
    const { rateLimitDirectiveTypeDefs, rateLimitDirectiveTransformer } = rateLimitDirective();


    const basicSchema = await buildSchema({
      authChecker: ({ context }, perms) => accessCheck(context, perms),
      resolvers: resolverClasses,
      container,
    });
    const schemaMergedWithTypes = mergeSchemas({
      schemas: [basicSchema],
      typeDefs: rateLimitDirectiveTypeDefs,
    });
    const schemaWithRateLimitDirective = rateLimitDirectiveTransformer(schemaMergedWithTypes);
    return schemaWithRateLimitDirective;
  }

...

Expected Behavior
When using "@ratelimit({ limit: 3, duration: 60 })" before a mutation, it should limit the rate at which the mutation can be called.

Logs
When building the schema through buildSchema:
Error parsing directive definition "@rateLimit({ limit: 3, duration: 60 })"

Environment (please complete the following information):

  • OS: Mac os, docker environment
  • Node 16-alpine.
  • Package version : 2.0.0.beta3,
    "graphql-rate-limit-directive": "2.0.4"
  • TypeScript version : 4.9.5
@MichalLytek
Copy link
Owner

Directives should be passed to the buildSchema function. TypeGraphQL performs schema check after build, so it does not know the definition of your directive and yells.

@MichalLytek MichalLytek added the Question ❔ Not future request, proposal or bug issue label Nov 11, 2023
@gyl89
Copy link
Author

gyl89 commented Nov 13, 2023

Thanks for your answers.

I feel thought that the documentation is not very clear :
https://typegraphql.com/docs/directives.html
Part on the 'fake-rename-directives" does not mention that. Besides, the package I mentioned does not export the directive but only the transformer and the typedefs.

@MichalLytek
Copy link
Owner

Fair point. Maybe @carlocorradini can tell you more as he wrote this chapter (based on git blame).

For now, try to use skipCheck: true option of buildSchema.

@carlocorradini
Copy link
Contributor

Sorry for the late reply, I'll check it ASAP 🥳

@carlocorradini
Copy link
Contributor

@gyl89
Strange... I'm using the same approach with graphql-auth-directive (see https://github.com/carlocorradini/graphql-auth-directive/blob/main/examples/typegraphql/index.ts)
Can you create an example repo with the code so we can examine and test it? Thanks 😅🥳

@MichalLytek MichalLytek added the Need More Info 🤷‍♂️ Further information is requested label Jan 6, 2024
@MichalLytek
Copy link
Owner

@gyl89 Can you provide a repository with a minimal reproducible code example, in order to debug the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue
Projects
None yet
Development

No branches or pull requests

4 participants
@MichalLytek @carlocorradini @gyl89 and others