Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-Authored-By: Glen <glen.84@gmail.com>
  • Loading branch information
bradzacher and glen-84 committed Jan 13, 2020
1 parent 96ebfa4 commit 8cd926d
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 10 deletions.
14 changes: 7 additions & 7 deletions packages/eslint-plugin/docs/rules/ban-types.md
Expand Up @@ -58,7 +58,7 @@ The rule accepts a single object as options, with the key `types`.
"@typescript-eslint/ban-types": ["error", {
"types": {
// add a custom message to help explain why not to use it
"Foo": "Don't use bar!",
"Foo": "Don't use Foo!",

// add a custom message, AND tell the plugin how to fix it
"String": {
Expand All @@ -77,18 +77,18 @@ The rule accepts a single object as options, with the key `types`.

### Default Options

The default options provides a set of "best practices", intended to provide safety and standardization in your codebase:
The default options provide a set of "best practices", intended to provide safety and standardization in your codebase:

- Don't use the upper case primitive types, you should use the lower-case types for consistency.
- Don't use the upper-case primitive types, you should use the lower-case types for consistency.
- Avoid the `Function` type, as it provides little safety for the following reasons:
- It provides no type-safety when calling the value, which means it's easy to provide the wrong arguments.
- It provides no type safety when calling the value, which means it's easy to provide the wrong arguments.
- It accepts class declarations, which will fail when called, as they are called without the `new` keyword.
- Avoid the `Object` and `{}` types, as they means "any non-nullish value".
- Avoid the `Object` and `{}` types, as they mean "any non-nullish value".
- This is a point of confusion for many developers, who think it means "any object type".
- Avoid the `object`, as it is currently hard to use due to not being able to assert that keys exist.
- Avoid the `object` type, as it is currently hard to use due to not being able to assert that keys exist.
- See [microsoft/TypeScript#21732](https://github.com/microsoft/TypeScript/issues/21732).

**_Important note:_** the default options suggests using `Record<string, unknown>`; this was a stylistic decision, as the built-in `Record` type is considered to look cleaner.
**_Important note:_** the default options suggest using `Record<string, unknown>`; this was a stylistic decision, as the built-in `Record` type is considered to look cleaner.

## Compatibility

Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/rules/ban-types.ts
Expand Up @@ -110,9 +110,9 @@ export default util.createRule<Options, MessageIds>({
Function: {
message: [
'The `Function` type accepts any function-like value.',
'It provides no type-safety when calling the function, which can be a common source of bugs.',
'It provides no type safety when calling the function, which can be a common source of bugs.',
'It also accepts things like class declarations, which will throw at runtime as they will not be called with `new`.',
'If you are expecting the function to accept certain arguments, you should explicitly define function shape.',
'If you are expecting the function to accept certain arguments, you should explicitly define the function shape.',
].join('\n'),
},

Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/tests/rules/ban-types.test.ts
Expand Up @@ -26,7 +26,7 @@ const options: InferOptionsTypeFromRule<typeof rule> = [

ruleTester.run('ban-types', rule, {
valid: [
'let f = Object();', // Should not fail, as it is not a type position
'let f = Object();', // Should not fail, as it is not in a type position
'let f = {};',
'let f: { x: number, y: number } = { x: 1, y: 1 };',
{
Expand Down

0 comments on commit 8cd926d

Please sign in to comment.