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

TypeGraphQL ESLint plugin #727

Open
borremosch opened this issue Oct 14, 2020 · 3 comments
Open

TypeGraphQL ESLint plugin #727

borremosch opened this issue Oct 14, 2020 · 3 comments
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request

Comments

@borremosch
Copy link

Is your feature request related to a problem? Please describe.
I am working as the lead in a team of software developers that have been working on a microservices solution backed by TypeGraphQL for almost two years now. Over time, I have come to find that a small number of errors related to TypeGraphQL decorators repeatedly keep popping up within our codebase. Two very common examples:

  • A develop forgets to add { nullable: true } inside a TypeGraphQL decorator for a variable or query that is typed as nullable. The opposite case is just as common.
  • A developer creates a number, supposed to contain an integer, but does not add a type function to the TypeGraphQL decorator, essentially making it a float over the GraphQL connection.

An additional problem with these types of errors is that they might not cause crashes or trouble right away, but months down the line when the written code is already in production and only vaguely remembered.

Describe the solution you'd like
I have been working on an ESLint plugin for type-graphql decorators. At the moment we have integrated this plugin in our 15 or so microservices that are using TypeGraphQL, and have been able to find around 20 cases where a mistake similar to the ones described above was made. These are errors that we have not been able to spot by eye before, despite having experienced engineers and peer-reviewed code.

I am hoping that more people are willing to try out this plugin, and report any bugs and suggestions they have so it can be further improved. Once the maturity level of the plugin is stable enough, I believe it can have a contribution to the adoption of TypeGraphQL. It might lower the learning curve, as some of the errors are non-obvious to developers that are new to TypeGraphQL.

For an overview of the currently supported linter rules, please see the documentation on the package page on the NPM website.

I am just dropping this here as a feature request as I did not find a more proper location to file this. Thoughts and suggestions are very welcome.

@MichalLytek
Copy link
Owner

The idea sounds great! I have no experience with eslint plugins development so I can't help with that for now.

While #296 can solve the nullability or missing explicit type issues, there's still a few cases where linter can warn or help during development, not only in runtime.

A developer creates a number, supposed to contain an integer, but does not add a type function to the TypeGraphQL decorator, essentially making it a float over the GraphQL connection.

How the eslint plugin would know, where the number property should be int and when should be float? 😕
Maybe your team should use decorator aliases like @IntField to prevent such hard to spot mistakes 😕

@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request labels Oct 17, 2020
@borremosch
Copy link
Author

While #296 can solve the nullability or missing explicit type issues, there's still a few cases where linter can warn or help during development, not only in runtime.

That is an interesting idea. Personally I would not like to use a precompiler to TypeScript as our build system is bloated enough right now, but the added value of such a concept is evident.

How the eslint plugin would know, where the number property should be int and when should be float? 😕

You are right, the plugin cannot know whether it should be an int or float. In the recommended settings, it will just warn when the type (either float or int) is not explicitly specified.

@MichalLytek
Copy link
Owner

MichalLytek commented Oct 19, 2020

Personally I would not like to use a precompiler to TypeScript

In the future, the typescript compiler plugin will be the default recommended setting, while the old, reflect-metadata based one with () => Type helpers will be a fallback, compatibility mode, mostly for babel users.

It's really easy to setup this - just one line in tsconfig. And it shouldn't be too slow because it will only put some runtime function call to collect type info, no advanced file rewriting or generating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants