This is pretty compelling. Why isn't it in the "recommended" preset of lints?
In general, I find it frustrating that the eslint recommended presets don't document why they are recommended. I disable several of the rules in all my projects because they seem to be arbitrary stylistic choices (e.g. [0], [1], [2]).
[edit] In looking up no-empty-function, I saw a stack overflow post that provides a compelling alternative of using `() => undefined` instead of `() => {}`, which suppresses the error. That should be shown as an example on the eslint page!
> I find it frustrating that the eslint recommended presets don't document why they are recommended.
That’s a good point. While this doesn’t address the root problem, I can share some context here as a former maintainer: browsing the core rules [1], you should see that recommended rules flag cases that are highly likely to be bugs or unnecessary constructs. Recommended rules should have false positives only in exceptional cases and be objective or near-universal consensus opinions.
Plugins, of course, are free to choose their own threshold for their recommended configs.
> Why isn't it in the "recommended" preset of lints?
It will be added to recommended in v9! [2] The rule was written during one of the v8 minor versions, and adding to the recommended config is always a breaking change.
0 is to prevent mistakes where you declare a function, but forget to implement it. If that's really intended I like to put a comment "do nothing" in empty functions, even in projects without the ESLint rule, so future readers know what's going on.
1 is for consistency & readability. Imagine you read someone else's code like <div children="foo" />, and you see that it's self-closing, so you expect it's an empty div. Even more confusing when you have more than just one attribute. Or say you want to modify the code to add a new child, so you remove the self-closing and add the child, breaking the old children in the process. What is the reason for doing children="foo" in the first place anyways?
2 is to prevent infinite loops, it's good practice to keep an upper bound (in a single-threaded world like JS, infinite loops can be very deadly and hard to diagnose). I like NASA's ten coding commandments (this is #2): https://devm.io/careers/power-ten-nasas-coding-commandments-...
Yeah I love rust’s todo!() macro for this. Not only does it throw an error, but it also typechecks in any context. Normally a function needs to actually return whatever it says it will return. But throw a todo!() in there, and you don’t need to return anything at all. (I assume there’s some type magic going on behind the scenes. Whatever it is, I love it.)
But generally I agree that it’s overly persnickety to complain about empty functions. My use case for them is that sometimes you need to pass a function as an argument and the function may be null - to indicate you don’t want to do any work. It’s often cleaner to use an empty function as a default value rather than add null checks everywhere.
I don’t use eslint at all because of defaults like this. Having my coding style negged by a tool feels awful. I hate gofmt. I ran it once and it deleted a bunch of empty lines in my code that I had put in on purpose to aid readability. And wow, that pissed me right off! I just know I’ll hate a lot of eslint’s rules just as much. 30 years of coding experience gives you opinions. Kids these days don’t even know how to use the ternary operator properly.
For being explicit that I’ve deliberately chosen to leave a function empty, I like to have a single noOp function defined that’s documented and use that to show I meant to do it. Only if you need it more than three times though of course!
This is correct. The post is from 2022. New default rules are a breaking change and version 9, the first major relates since it was added, is coming soon and will include it by default.
From what I remember, being able to pass children as a prop is considered a side-effect of an implementation detail, that breaks the expected abstraction. There really isn't any reason to use it, and I think there's a chance it may even limit what the virtual dom diffing can manage?
Also this would prevent you from accidentally doing both at once:
<MyComponent children={<div>Is it me?</div>}>
<div>Or is it me?</div>
</MyComponent>
(I don't remember what React does in this situation)
I use it fairly extensively whenever I have code like <Cpt>{props.children}</Cpt> (i.e. whenever the only thing I'm doing with the children prop is passing it to another component. This comes up pretty often with context providers, where I'll write a provider wrapper that handles some stuff by itself, and then passes the value and children to a React context.
I like using the "children" prop explicitly because it's a lot more concise (with prettier, I can often go from five lines to one line with this style), and because it's more explicit. I'm not creating my own children, I'm not adding anything to the markup myself, I'm just passing the input children directly to another component.
This isn't really anything unusual, it's exactly what JSX compiles to under the hood. And passing JSX elements in props rather than as children is useful in other contexts as well - for example, a button that allows <Btn icon={<MyIcon />} ...>
You're right that this can cause odd situations where children are specified in two ways, but I'd rather have a lint rule explicitly handle this case than disallow using the "children" prop altogether. (In fact, I think Typescript does validate this case already? But I might be wrong there.)
I can see why this rule might be useful to have in general, but I agree with the previous poster that it's a semi-opinionated rule, and one of the sort of rules where I end up having to disable it and fiddle around with configs whenever it comes up, which in turn puts me off using ESLint in general because I know to get it to be useful I'm going to need to spend some time changing everything up.
In general, I find it frustrating that the eslint recommended presets don't document why they are recommended. I disable several of the rules in all my projects because they seem to be arbitrary stylistic choices (e.g. [0], [1], [2]).
[0] https://typescript-eslint.io/rules/no-empty-function/
[1] https://github.com/jsx-eslint/eslint-plugin-react/blob/maste...
[2] Specifically checkLoops of https://eslint.org/docs/latest/rules/no-constant-condition
[edit] In looking up no-empty-function, I saw a stack overflow post that provides a compelling alternative of using `() => undefined` instead of `() => {}`, which suppresses the error. That should be shown as an example on the eslint page!