Javascript: No rule for “Hoisting”

[architectcodes]

The section “Hoisting” does a good job of explaining the ins and outs of hoisting, but states no rule about that.

For example I consider this a bad practice, because it leads to confusion and possible bugs:

function example() {
  superPower(); // => Flying

  function superPower() {
    console.log('Flying');
  }
}

[goatslacker]

Why do you consider it bad practice?

[architectcodes]

I'm used to reading code top-to-bottom – and function hoisting is the only thing in JavaScript which breaks that rhythm. I sometimes find myself confused, searching for an overlooked require while the function I'm looking for is declared at the very end of a module.

Anyway, this is just my personal opinion. But – as I see it – the purpose of this guide is to explain problems and express opinions on them. All other sections come with opinions, and “Hoisting” only comes with an in-depth explanation.

[ryan-codingintrigue]

I agree with the fact this should be addressed, not because declaring functions in a certain way is "bad practice" - it isn't - but for code consistency. Personally I like declaring the functions where they make the most sense in terms of the code, safe in the knowledge they'll be hoisted. My IDE will point me in the right direction if I need to locate it.

Just seems odd that you would explain it, but not offer any style guidelines one way or the other.

[architectcodes]

By the way, my original example results in an error with your official setup for _jsHint_:

line 4, col 22, 'superPower' was used before it was defined.

Here’s the culprit: https://github.com/airbnb/javascript/blob/f33e85e/linters/jshintrc#L35.

You could overcome it by setting "latedef": "nofunc". But I’d cast my vote for leaving it as it is and updating the rules :)

[gregersrygg]

I'm used to reading code top-to-bottom – and function hoisting is the only thing in JavaScript which breaks that rhythm.

I'd say it's the rather opposite. With this rule you have to read the code backwards:

function c() {}
function b() { c(); }
function a() { b(); }
a();

Instead of:

a();
function a() { b(); }
function b() { c(); }
function c() {}

As far as I know, there are no side-effects of hoisting named function definitions, and it gives you the ability to have functions ordered in a more natural order. So my vote (if I have one :monkey:) goes for relaxing this rule with "nofunc".

The section for Functions is also kind of meaningless when it's not allowed:

Why? Function declarations are named, so they're easier to identify in call stacks. Also, the whole body of a function declaration is hoisted, whereas only the reference of a function expression is hoisted.

[ljharb]

"backwards" and "more natural order" is the subjective part. In my view, it's backwards to see a token before you've read its definition - in my own code, I never rely on hoisting, and consider doing so a bug.

With respect to function declarations versus named function expressions, certainly a rule to prohibit hoisting lends itself towards prohibiting declarations (a rule I also strictly follow in my own code), but there's nothing wrong with using a function declaration, and only referencing it lexically after it's declared.

[nkbt]

@ljharb :+1:
I use no-use-before-define ESLint rule for that.

[architectcodes]

I agree with @ljharb. Your approach to “backwards” vs “forwards” may be as subjective as others, but it’s much more scalable.

I mean, these would be illegal – even if more natural for fans of hoisting:

a();

import a from 'a'; 

a();

const a = () => {}; 

[szimek]

As mentioned before, the code example in 14.4 raises an error with provided config for ESLint. One can use "no-use-before-define": [2, "nofunc"] to allow only function declarations, though in ES6 where let and const are preferred, "no-use-before-define" is probably not that important anymore.

In Angular we do something like this to register controllers and other stuff:

angular.module("app").controller("MyController", MyController);

function MyController() {
  ...
}

that's why we have this rule set to "nofunc".

[ljharb]

The no-use-before-define rule prevents hoisting, as does the guide itself. I think this can be closed.

[jesucarr]

What about nested calls? For example:

const type = new Type({
  resolve: resolveAorB,
});
const a = {
  type: type,
};
const b = {
  type: type,
};
function resolveAorB(condition) {
  if (condition) return a;
  return b;
}

To comply with the rule I need to declare the function as variable at the top with some random value, and then redeclare it at the bottom when all the variables are being declared.

let resolveAorB = undefined;
const type = new Type({
  resolve: resolveAorB,
});
const a = {
  type: type,
};
const b = {
  type: type,
};
resolveAorB = (condition) => {
  if (condition) return a;
  return b;
};

This seems less elegant than letting JS do the hoisting for me. Are we supposed to solve it like this, or is there any other solution that I'm missing?

[ljharb]

@jesucarr That solution won't actually work, because undefined would be passed in, not the later value of the function. The trivial solution is adding // eslint disable-line no-use-before-define on the function resolveAorB line.

However, it's likely that your implementation and use of Type could be redesigned such that this wasn't a problem. One example could be, type being a thunk, instead of a data property. I'm not really sure since I don't have all of your requirements and codebase in my head :-)

[jesucarr]

@ljharb you are right, in the actual implementation it will go inside a function so when called will have the right value. Overlooked that when doing the code simplification.

The design is based on the graphql interface type, like here, so I can't redesign it.

But thanks for the clarification.