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

fix: change the empty object type {} to use TS Record utility type #21

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

aniravi24
Copy link
Contributor

@aniravi24 aniravi24 commented May 20, 2022

To use the more accurate type based on typescript/eslint's recommendations, The generated type when there's no param was changed to Record<string, never>

 Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead

https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/ban-types.md

For example, this is valid in the current main branch

image

But once it's changed, now it throws an error

image

However, this is still valid as expected

image

@aniravi24
Copy link
Contributor Author

@sandulat we enabled a plugin in our project that triggered this error, thought I should change it here so it works for everyone! The Record utility type has been around since TS 2.1 so it should work.

@IgnisDa
Copy link

IgnisDa commented Jul 8, 2022

@sandulat Can this be merged please?

@sandulat sandulat self-requested a review July 8, 2022 15:10
@sandulat sandulat merged commit df5c3aa into sandulat:main Jul 8, 2022
@sandulat
Copy link
Owner

sandulat commented Jul 8, 2022

@sandulat
Copy link
Owner

sandulat commented Jul 8, 2022

Thank you very much @aniravi24!

@aniravi24
Copy link
Contributor Author

@sandulat awesome, thank you!

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 this pull request may close these issues.

3 participants