Javascript: no-param-reassign and no-use-before-define

[EddyVinck]

So I just spent a few hours setting up ESLint for the first time with the AirBnB settings and let me just say that it was worth it. My code looks much nicer and it helped me find some improvements.

the rules no-param-reassign and no-use-before-define still need to be followed in the code but I'm not sure if I agree with those rules, or maybe I don't understand how to deal with them yet.

no-use-before-define makes it so you cannot follow the 'top-to-bottom' principle from Clean Code because it complains when you do:

const higherLevelFunction = () => {
       // stuff

       lowerLevelFunction(); 
       // [eslint] 'lowerLevelFunction' was used before it was defined. (no-use-before-define)

       // more stuff
};
const lowerLevelFunction = () => {
       // compute something
};
// Bottom of file...
higherLevelFunction();

no-param-reassign makes it so you cannot do stuff like:

someNodeList.forEach((el) => {
      el.style.display = 'none';
});
// [eslint] Assignment to property of function parameter 'ct'. (no-param-reassign)

Are these things intentional? And if they are, how can I comply with these rules?

[ljharb]

I'm not familiar with the principle you're talking about, but yes, you're supposed to see the definition (or the import/require) of things before you use them. Relying on hoisting makes code harder to maintain, and so no-use-before-define is critical in ensuring that.

As for no-param-reassign - we don't run into the situation you're talking about because we use React, but for that scenario it makes sense to use an eslint override comment.

[EddyVinck]

Here is a link to Clean Code JavaScript section where they discuss the principle I'm talking about: https://github.com/ryanmcdermott/clean-code-javascript#function-callers-and-callees-should-be-close

I might be mistaken, but I don't think this code relies on hoisting. I ran it in the console and it worked fine because b is defined when a is invoked.

const a = () => {
    b();
}
const b = () => {
    console.log("Hello from B!");
}
a(); // -> "Hello from B!"

This, of course, does not run:

const a = () => {
    b();
}
b(); // Uncaught ReferenceError: b is not defined at <anonymous>:4:1
const b = () => {
    console.log("Hello from B!");
}
a();

[ljharb]

I agree that your example isn't relying on hoisting; however that's still code that the user will see used, as they read top to bottom, before it's defined - harming readability. In other words, before b is invoked, i should know what b is.

[tunnckoCore]

@ljharb harming readability

absolutely not.

The "from top to bottom" is exactly about that. When you getting started with some package or code, you open the file, starting to read from the top (or search where the module.exports is) then reading line by line you are seeing bar(), ooh okey, you scroll down and the next function declaration is exactly what you need. When you finish reading the bar you eventually goes up to where you first found it, or continue the same way to the bottom.

This "principle" is absolutely stunning and is always helpful and everyone should trying to follow. I'm doing this for years and reading this way for years, and it is a huge boost.

Start reading this for example https://github.com/olstenlarck/hela/blob/v3-major/packages/core/src/index.js#L214-L219 (the main thing is the action() method). One may say "you start from the bottom", no, it's just bigger convention here in nodejs, but not applies to another languages.

You start reading the action() method, you seeing createHandler() cal, but also visually noticed (maybe i'm the freak, cuz have amazing photographic memory) that in few lines later there is also stringActionWrapper() - okey, scroll down - ha! it's exactly after the hela() declaration. You started reading createHandler() and you immediately seeing one more call to stringActionWrapper(), so you think _hm, okay i'm going to this after i finish with that_. But hA! wat a surprise!? It's exactly the next function declaration. After you read the stringActionWrapper declaration, you noted the handleChaining declaration, and you going above back to where you start - the action() method, you are seeing handleChaining() and createActionWrapper() so you go to them and you are ready.

That's why i willl always use function declarations instead of some funky new syntaxes and ways like const foo = () => {}.

This rule is absolutely great. That's why i always change it a bit

'no-use-before-define': ['error', { 
+ functions: false, 
  classes: true, 
  variables: true }]
'no-use-before-define': ['error', { 
- functions: true,
  classes: true,
  variables: true }]

[EddyVinck]

Related: https://github.com/eslint/eslint/issues/10589

[ljharb]

@olstenlarck I find that to be the opposite in my experience.

This guide will continue strictly forbidding using anything before it's defined.

Closing, since this is answered.

[ericblade]

@olstenlarck those are two completely conflicting code philosophies, so you may want to disable that in your local eslint conf. There might even be some eslint configuration that allows you to specify that you want the style you prefer, also, but i'm not familiar enough with all the options to know.

[tunnckoCore]

@ericblade i know and i'm doing it as shown above.

[dtinth]

Adding this comment just for reference, so people don’t feel too guilty disabling the no-use-before-define rule.

---

Here’s an example case where we had to disable the rule no-use-before-define. This came from one of our scripts doing some deployments:

function main() {
  deployApplication();
  invalidateCDNCache();
  verifyDeployment();
  notifyTeam();
}

function deployApplication() {
  deployAssetFiles();
  deployIndexFile();
}

function deployAssetFiles() {
  /* 20 lines of code */
}

function deployIndexFile() {
  /* 20 lines of code */
}

function invalidateCDNCache() {
  /* 30 lines of code */
}

function verifyDeployment() {
  /* 15 lines of code */
}

function notifyTeam() {
  /* 10 lines of code */
}

main();

In terms of readability, having main at the top means that a reader can understand what the script does by reading just the first 6 lines. Compare it to this:

// 🤔 Where will this be used? Will have to read more to find out...
//    1 thing to keep track on my head.
function deployAssetFiles() {
  /* 20 lines of code */
}

// 🤔 Where will this be used? Will have to read more to find out...
//    2 things to keep track on my head.
function deployIndexFile() {
  /* 20 lines of code */
}

// 🤔 Where will this be used? Will have to read more to find out...
//    3 things to keep track on my head.
function deployApplication() {
  // 🤔 Oh, they are used here! 1 thing left to keep track of.
  deployAssetFiles();
  deployIndexFile();
}

// 🤔 Where will this be used? Will have to read more to find out...
//    2 things to keep track on my head.
function invalidateCDNCache() {
  /* 30 lines of code */
}

// 🤔 Where will this be used? Will have to read more to find out...
//    3 things to keep track on my head.
function verifyDeployment() {
  /* 15 lines of code */
}

// 🤔 Where will this be used? Will have to read more to find out...
//    4 things to keep track on my head.
function notifyTeam() {
  /* 10 lines of code */
}

function main() {
  // 🤔 Oh, they are used here!
  deployApplication();
  invalidateCDNCache();
  verifyDeployment();
  notifyTeam();
}

main();

People don’t put table of contents at the end of the book. I believe the same principle applies to code as well.

---

I noticed that the rule is not documented in the README file… would be great if the rules and rationale are also documented there.

[ljharb]

@dtinth the advice would be to extract all those things into separate files and require them in - if you don't need to read the implementations first, they shouldn't be in the file. The table of contents is all the requires/imports, not the main logic of your application.

[tunnckoCore]

@dtinth, exactly, thanks for the visual example of exactly what I'm talking about.

@ljharb: the advice would be to extract all those things into separate files and require them in

Even then. But consider that the whole thing is just 150 lines. Mine, usually, are 150-200 _WITH_ the docblock comments. And yes, I sometimes separate them.

[ljharb]

Separate files are great even if the function is, say, 3-5 lines - with or without comments (comments are a subject for another thread).