-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Bug: [prefer-reduce-type-parameter] unfixable reporting #3440
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
Both of your examples are completely unsound due to the const str = normalizeParams('foo');
// typeof str === 'foo'
// but at runtime
// str === {0: 'f', 1: 'o', 2: 'o'}
const num = normalizeParams(1);
// typeof num === 1
// but at runtime
// num === {} const str = toKeys('foo');
// typeof str === string
// but at runtime
// str === {0: "0", 1: "1", 2: "2"}
const num = toKeys(1);
// typeof num === number
// but at runtime
// num === {} Doing an |
Then what about adding export function normalizeParams<P extends object>(params: P): P {
return Object.entries(params).reduce((acc, [key, value]) => {
value = typeof value === 'string' ? value.trim() : value;
if (value) {
acc[key as keyof P] = value;
}
return acc;
}, {} as P);
} Or, do you have any recommendation to handle these cases? |
It's hard to say exactly - TS does treat generics kind of weirds in conjunction with empty object types. THere's no way to write this function without using an |
Actually I respectly disagree with
It is very useful when you want a generic which must be an Object, not primitive nor Array, there is no alternative, the suggestions |
I actually just hit this in the typescript-eslint codebase 😄: typescript-eslint/packages/eslint-plugin/src/rules/naming-convention-utils/parse-options.ts Lines 82 to 90 in 9e35ef9
type ParsedOptions = Record<'a' | 'b', number>;
function parseOptions(): ParsedOptions {
const result = ['a', 'b'].reduce((acc, k) => {
acc[k] = k;
return acc;
}, {} as ParsedOptions);
return result;
} @bradzacher @JounQin what was the fix you had in mind? Adding an option to disable Note that we're blocked from actually detecting unfixable reporting on my favorite TypeScript issue: microsoft/TypeScript#9879. 😢 |
Potential fixes:
const result = ['a', 'b'].reduce<ParsedOptions>((acc, k) => {
acc[k] = k;
return acc;
}, {} as ParsedOptions);
|
The big problem you've got is this code is straight-up unsound though - it's not good code! For example Josh your code has There's also the problem of missing keys - your type specifies the object must have the keys
|
Now that v8 is released, per #7936, we can mark this issue as unblocked! 🚀 |
Repro
Expected Result
Ignore such cases
Actual Result
Error reporting
Additional Info
Versions
@typescript-eslint/eslint-plugin
4.24.0
@typescript-eslint/parser
4.24.0
TypeScript
4.2.4
ESLint
7.27.9
node
12.22.1
The text was updated successfully, but these errors were encountered: