Skip to content

Enhancement: [ban-types] Suggest NonNullable<unknown> as an alternative for {} and Object #6486

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

Closed
4 tasks done
arieljbirnbaum opened this issue Feb 17, 2023 · 2 comments · Fixed by #6876
Closed
4 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@arieljbirnbaum
Copy link

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/ban-types/

Description

The rule forbids the use of the type {} (and its synonym Object), which means "any non-nullish value", and gives some helpful suggestions on what to use instead, depending on what the developer actually might have meant.

However, there seems to be no suggestion (outside disabling the rule) for what to use when the intended meaning really is "any non-nullish value". The suggested alternative for "any object", namely object, doesn't include primitive types such as number, so it's not an equivalent replacement in this case.

Rather than bypassing or disabling the rule, and leaving the easily misunderstood {} in place, I think it would be useful to suggest an explicit alternative for this case as well; something along the lines of

- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead.

(placed after all the other suggestions).

TypeScript includes the NonNullable utility type since 2.8, so I don't think there shouldn't be any compatibility issues.

PS: I wasn't sure if this issue should go under Documentation or Enhancement; since it changes the output of the rule I opted for the latter. My apologies if I got it wrong :)

Fail

/**
 * This is a generic combinator to add a fallback value to a function, to handle a null input.
 * However, we don't want to accidentally use it when the function _already_ handles null,
 * otherwise our fallback will clobber the original value.
 */
const withFallback: <Input extends {}, Output>( // This fails the rule and that's OK :)
  mapping: (input: Input) => Output,
  fallback: Output
) => (input: Input | null) => Output = (mapping, fallback) => (input) =>
  input === null ? fallback : mapping(input);

const plusOne: (input: number) => number
const plusOneWithFallback = withFallback(plusOne, 0); // This won't compile if we replace {} with object

Pass

const withFallback: <Input extends NonNullable<unknown>, Output>( // Same meaning, explicit, passes :)
  mapping: (input: Input) => Output,
  fallback: Output
) => (input: Input | null) => Output = (mapping, fallback) => (input) =>
  input === null ? fallback : mapping(input);

const plusOne: (input: number) => number
const plusOneWithFallback = withFallback(plusOne, 0); // This compiles

Additional Info

No response

@arieljbirnbaum arieljbirnbaum added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Feb 17, 2023
@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Apr 8, 2023

👋 @grumpygreenguy, sorry for the delay here! This seems reasonable to me (and @Josh-Cena also 👍d). Accepting PRs to add the new line to the end of the existing output for the rule.

And yes, you filed this issue correctly 😄 thanks!

@JoshuaKGoldberg JoshuaKGoldberg added good first issue Good for newcomers accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Apr 8, 2023
@arieljbirnbaum
Copy link
Author

@JoshuaKGoldberg No worries; thanks to you and @NotWoods for picking this up! 🙌

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
2 participants