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

Don't disable unicorn/no-null? #69

Closed
justinhelmer opened this issue Nov 30, 2022 · 0 comments
Closed

Don't disable unicorn/no-null? #69

justinhelmer opened this issue Nov 30, 2022 · 0 comments

Comments

@justinhelmer
Copy link

justinhelmer commented Nov 30, 2022

Background
I like your position on not using null (in favor of undefined).

This library does two things:

  1. Adds a @typescript-eslint/ban-types rule that disables the use of the null type.
  2. Turns off the unicorn/no-null rule in favor of @typescript-eslint/ban-types.

For a little more background, we were led to your xo and xo-typescript configurations by using npm init @eslint/config from ESLint's Getting Started (and we love it). I stumbled on the rule of disallowing the null type by accident, read your discussion, and fully bought in.

Problem
The problem is, that @typescript-eslint/ban-types will not match value usages, by design. However, the unicorn/no-null rule specifically disallows the use of the null literal:

// will yell
const param: null = null;

// will not yell
const param = null;

Since we want to use type inference as much as possible, the current @typescript-eslint/ban-types rule in this library probably won't catch much for us - we need the value checks.

Suggestion

  1. Don't disable the unicorn/no-null rule since it complements the @typescript-eslint/ban-types rule (one checks literals, one checks types).
  2. Make this behavior the default when using npm init @eslint/config (maybe include eslint-plugin-unicorn?) - I realize this would be a separate PR in a separate repo.

For (1), we currently have the following config in the meantime (reduced to only show the important bits):

module.exports = {
  extends: [
    'plugin:unicorn/recommended',
    'xo',
    'xo-typescript'
  ],
  rules: {
    'unicorn/no-null': 'error',
  },
};

For (2), you will probably tell us to just use XO directly instead of juggling the extends ourselves.

WDYT?

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

No branches or pull requests

1 participant