-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[ban-types] Conflict with no-explicit-any #842
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
Comments
It's not a good idea to use Now a days, this should instead fix to How come you're writing such a loosely typed function? There's almost always better options. |
`Record<string, any>` is not typesafe, so we shouldn't suggest it as the default fixer. Additionally, using `any` causes errors via the `no-explicit-any` rule. `Record<string, unknown>` is safe because you have to use type guards to actually use the value. Also add handling for `object`; it wasn't added because I didn't think about the fact that it's not fixing `Object` -> `object` like the rest of the fixers. Fixes #842
How is However, I'm wondering if any of these options are actually appropriate, since they narrow the type which could be a breaking change. Would it not be best to simply not provide a fixer for this case, and rather just have it be a suggestion that the developer consider a more type-safe alternative?
Again, more restrictive, but ultimately not a good type to use in most cases, so there's little point in fixing to it IMO.
It's not my code – I'm adding linting to an existing project. There are cases where methods are returning |
In testing, it looks like const x: object = { a: 1 };
x.foo // error - key foo not in x
const key = 'a';
x[key] // error - no string index on x
if ('a' in x) {
// typeof x === never
x.a // error - key a not in type never
}
const y: { a: 1 } = x; // error - cannot assign {} to {a: 1}
const z = x as { a: 1 }; IMO const x: Record<string, unknown> = { a: 1 };
x.foo // typeof === unknown
const key = 'a';
x[key] // typeof === unknown
if ('a' in x) {
x.a // typeof === unknown
}
const y: { a: 1 } = x; // error - cannot assign Record<string, unknown> to {a: 1}
const z = x as { a: 1 };
const y1: object = {};
const y2: object = new Map();
const y3: object = 1; // error
const y4: object = ''; // error
const y5: object = false; // error
const y6: object = [];
// no errors at all!
const x1: Object = {};
const x2: Object = new Map();
const x3: Object = 1;
const x4: Object = '';
const x5: Object = false;
const x6: Object = []; I'm struggling to understand why you'd ever use edit - reading into it more, it looks like unknown is essentially If the observable requires something which you don't care about the value of, why not define the observable as having a default value for the generic |
Did you mean unusable?
You might be right, but I think it's best to leave this decision to the developer, who may have specific reasons for choosing between the three options (or better, a more specific type).
I never said that I wanted to use
Well if your theory is correct, the answer is there – the developer may not have wanted to allow null or undefined.
Ya, it's an rxjs thing. If we make the method itself generic, then that could perhaps be the default type (but then it's probably best for it to actually not have a generic default). |
Sorry by usable I mean that you can actually operate on, and use const key = 'a';
const x1: object = { a: 1 };
x1.a // error key a not on x
x1[key] // error no index accessor on x
if ('a' in x1) {
x1.a // error key a not on type never
}
const x2 = x1 as { a: 1 };
// now I can actually do the above things, but there's no type safety afforded by this cast, i.e.:
const x3 = ({ } as object) as { a: 1 }; // no errors
// compared to
const y1: Record<string, unknown> = { a: 1 };
y1.a // works with type unknown
y1[key] // works with type unknown
if ('a' in y1) {
y1.a // works with type unknown
}
const y2 = y1 as { a: 1 };
// note that this unsafe explicit assertion still works without error, so this use case is no more safe
const y3 = ({ } as object) as Record<string, unknown>; |
Yes, I understood, but you were referring to (here as well [ |
woops - sometimes I forget to proof read my comments after I restructuring the sentences.. |
No activity on this issue even though this is still open? Shouldn't the autofix logic change Object to Record<string, unknown> instead of Record<string, any> (which causes a further type error)? |
See #848. The fixer will be removed. It's considered to be a breaking change because |
We'll be planning to start on the 3.0 release soon™. |
`Record<string, any>` is not typesafe, so we shouldn't suggest it as the default fixer. Additionally, using `any` causes errors via the `no-explicit-any` rule. `Record<string, unknown>` is safe because you have to use type guards to actually use the value. Also add handling for `object`; it wasn't added because I didn't think about the fact that it's not fixing `Object` -> `object` like the rest of the fixers. Fixes #842
Repro
Expected Result
Error on
params
forban-types
, fixes to {} or suggests object, or at least something that doesn't include an explicitany
.Actual Result
Error on
params
forban-types
, fixes toRecord<string, any>
which conflicts with theno-explicit-any
rule.Additional Info
n/a
Versions
@typescript-eslint/eslint-plugin
1.13.0
@typescript-eslint/parser
1.13.0
TypeScript
3.4.5
ESLint
6.1.0
node
10.16.0
npm
6.9.0
The text was updated successfully, but these errors were encountered: