-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Ignoring React components defined as an arrow function #35
Comments
I think this proposal can be generalized. I think arrow functions are fine when assigned to a variable with a type declaration. type Predicate = (value: unknown) => Boolean:
const isNullish: Predicate = (value) => value == null; |
I don't plan to add type parsing to these rules. That would need to be another rule that someone else could build. |
I think the rule does not necessarily need to parse the type, it just needs to know if a variable has an explicit type declaration. To me, this seems like a reasonable exception for the options |
It's too slippery of a slope. We add an exception for "if a type is present" and then someone wants "well, if the type is like this __". I am opposed to TS personally, so I'm not motivated to cross the barrier into making this rule type-aware in any sense. |
More generally, my reaction to requests to add exceptions for the But the more I think exceptions should be handled as exceptions rather than as rule configurations. Add an eslint-disable comment if you really feel like using an arrow in a place this lint rule doesn't allow you to. With respect to the OP specifically, I would suggest just disabling "export" mode, with |
To address this opinion, explicitly: I respect your opinion here, but I very much disagree with it. In fact, I'd say it's counter to the underlying philosophy of this rule. The more is involved in an So making an exception mode for allowing "arrow functions with types" is going in the opposite direction to what this rule is trying to accomplish, which is that it's trying to allow only simpler (visually) arrow functions and it's trying to discourage complex (visually) arrow functions. |
In this specific case, TypeScript users are pretty much forced to use function expressions if they want to reuse function types. Given function types can be quite complex or even opaque, it can be very inconvenient to not use them. The inability to declare the type of a function declaration is less of an ad-hoc exception, rather it is a fundamental limitation of TypeScript syntax. I myself would prefer if TypeScript added syntax to declare the type of a function declaration, perhaps like this: // Unfortunately not possible in TypeScript
function isNullish: Predicate (value) {
return value == null;
} I guess one could also use function expressions instead of arrow functions. Would const isNullish: Predicate = function (value) {
return value == null;
}; I understand not wanting to make this package TypeScript-specific at all, it really is a slippery slope. The reality is that more and more people are using TypeScript though. Perhaps someone needs to make a TypeScript-aware variant of the rules - I have seen this for other rules that can be improved/modified due to the extra information afforded by types. |
This is legal in TS, AFAICT: function isNullish(value: unknown): Boolean {
return value == null;
} I understand that's not a "reusable" function type signature. But that's just a tradeoff that TS forces. Moreover, IMO, function return types should be explicit on the function rather than generically re-used. |
Of course it is possible to just declare the type of the function inline. In this contrived example, this is probably fine - and it will compile without issues due to TypeScript having a structural type system. However, I would argue there are definitely cases where it can be advantageuous to reference named function types. The name can communicate meaning, and the reusability aspect can reduce overall complexity and code duplication. I am curious what you think about the second example I provided in #35 (comment). I like having the keyword |
If I understand you correctly, you're asking about this function form? const isNullish: Predicate = function (value) {
return value == null;
}; That's not an arrow function, so no part of this lint rule will ever say anything about it. |
While I agree with you, I would still prefer more pragmatic approach. @spawnia very nicely explained why in some places it is really useful to reuse function type aliases (typing arguments and returns is repetitive and can lead to subtle erros). I can't avoid using them, but in other cases I would still like to enforce using function declarations. |
Several features/modes of this rule-plugin have been added after extensive discussion and lobbying by users. I am pragmatic enough to be willing to consider and discuss any ideas. But there's a core philosophy driving this project, and I don't think it serves anyone well to water that core philosophy down, at least not without a compelling argument. Thus far, "I want.." and "It'd be nice to.." style justifications have been advanced. I don't feel those overcome the philosophical objection I have to adding such a rule-exception. Yet. I have firmly said I don't have any intention to make this rule "type aware", but beyond that, I haven't given an absolute "no". Just a, "this doesn't convince me." |
FWIW, I think TypeScript 4.9 |
Unfortunately |
...yet. Looks like TS unfortunately dropped the ball on that one. |
Would it be possible to ignore React components in the rule
where
? While I agree that function declaration is preferable when exporting a function (i.e.export function somefn() { /* ... */}
), I would like to make exception for React components, because arrow functions are typed more easily.When using function declaration, you have to type props and also return (from my experience, people usually forget to add
null
):Using arrow function is little bit more concise, less error prone, but still readable thanks to the
React.FC
type right after the variable name:Thanks for the info.
The text was updated successfully, but these errors were encountered: