The issue has been solved
A function definition should be placed as close to the top level scope as possible without breaking it's captured values.
Fail
export function create(context) {
// does not capture anything from the scope, can be moved to the outer scope
function isSpecial(node) {
return node.type === 'Special';
}
return isSpecial;
}
Pass
function isSpecial(node) {
return node.type === 'Special';
}
export function create(context) {
return isSpecial;
}
export function create(context) {
function isSpecial(node) {
return node.type === 'Special' && context.isSpecial(node);
}
return isSpecial;
}
Sounds good. I often have to manually comment about this in PR reviews.
Slightly off-topic, but I would like the inverse for variables. That they are placed in the most inner scope as possible. What do you think?
This might actually be perfectly on-topic, because of the interaction between the two rules.
In this example, should context be moved in, or should isSpecial be moved out? (I have no idea yet)
const context = undefined;
export function create() {
function isSpecial(node) {
return node.type === 'Special' && context.isSpecial(node);
}
return isSpecial;
}
On a second thought, moving variables in may be a bad idea because it may negatively affect performance.
Yeah, let's keep this focused on only functions for now. And yes it should be moved out.
@issuehunt has funded $80.00 to this issue.
I have a prototype for this working; hope to have a PR out in the next couple days.
Alternative rule names:
- function-scoping
- function-lifting
- function-hoisting
@sindresorhus has rewarded $72.00 to @mrhen. See it on IssueHunt
- :moneybag: Total deposit: $80.00
- :tada: Repository reward(0%): $0.00
- :wrench: Service fee(10%): $8.00
IssueHunt