sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.32k stars 366 forks source link

`switch-case-braces`: Please change the default to `avoid` #2415

Open fregante opened 4 months ago

fregante commented 4 months ago

I've come to dislike switch because it's become so noisy and verbose over plain if/else, even if I have to repeat the condition multiple times.

This:

if (event.key === 'Tab') {
    showExtras = true;
} else if (event.key === 'ArrowDown') {
    focusNext('.ext-name, [type="search"]');
} else if (event.key === 'ArrowUp') {
    focusPrevious('.ext-name, [type="search"]');
} else {
    return;
}

Turns into:

switch (event.key) {
    case 'Tab': {
        showExtras = true;
        break;
    }

    case 'ArrowDown': {
        focusNext('.ext-name, [type="search"]');
        break;
    }

    case 'ArrowUp': {
        focusPrevious('.ext-name, [type="search"]');
        break;
    }

    default: {
        return;
    }
}

The latter is arguably more readable/airy, but so is this:

switch (event.key) {
    case 'Tab':
        showExtras = true;
        break;

    case 'ArrowDown':
        focusNext('.ext-name, [type="search"]');
        break;

    case 'ArrowUp':
        focusPrevious('.ext-name, [type="search"]');
        break;

    default:
        return;
}

I don’t think that the braces add anything here.

fisker commented 4 months ago

I use avoid too, but Sindre prefer alway https://github.com/sindresorhus/eslint-plugin-unicorn/pull/1709#issuecomment-1030263041

sindresorhus commented 4 months ago

It's done for consistency. I feel it would look weird and inconsistent to only have braces when you need to use const inside a case.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/switch#lexical_scoping