Think about using `const` as much as possible in Torbutton code

added TorBrowserTeam202006 component::applications/tor browser gitlab-tb-torbutton owner::tbb-team points::1 priority::medium severity::normal status::new tbb-torbutton type::task labels

I am not sure if we should use const everywhere we can. Is Mozilla doing this? Here is an article that makes for interesting reading; especially look at the "Liberal let" and "Constantly const" sections: https://madhatted.com/2016/1/25/let-it-be

What to do probably comes down to personal preference, but as a team it would be good to adopt some guidelines.

Hi I also like this article https://mathiasbynens.be/notes/es6-const (TL;DR const doesn't allow rebinding [it is not about immutability]).

I agree with mcs about in the end the technical advantages are small, however for new contributors it reduces the cognitive load, because they know that variable will not change in that scope.

E.g Every time I see a let in the beginning of the file, I assume that variable can change and it can be undefined or null!

For me, defaulting to const where it makes sense (ie not for obviously mutable variables like iterators) reduces the cognitive load during development and when re-visiting code later. Seeing that something is marked const lets me know that when the reference is used it is pointing to the same object throughout the block. I also appreciate the runtime/build errors when trying to reassign a const variable, especially given that JS is not strongly typed.

My general view on these types of things is that the earlier in the pipelinee I get an error the better. Using const helps protect me from a certain class of 'whoops overwrote the reference' bugs, so I'll probably be using it where I can.

Going forward I'd be in favor of preferring const everywhere in new code in Tor Browser.

Is Mozilla doing this?

I think most of the Firefox JS code uses let for declarations, but I did not see any general guidelines for this. Some components seem to be enforcing const though, like devtools: https://bugzilla.mozilla.org/show_bug.cgi?id=1454696.

I think using const as much as possible does add value if it's clear what it means, like pospeselr said: when the reference is used it is pointing to the same object throughout the block. It does not mean that the object of a const declaration cannot change. So I would say +1 to the idea of using const whenever it's possible and let otherwise.

I read the article mcs mentioned, I think it has some interesting points. Giving my opinion about some of them:

...

It adds an extra distraction to the process of programming...

I think this is true, but I also think that the additional distraction is quite small.

...

...and results in code difficult to understand and change.

I don't see how it's more difficult to understand, but I guess this is subjective. I think being able to easily see whether the value of a variable will remain the same for the whole function cannot make the code more difficult to understand. It probably does make the code more difficult to change, but not sure that's a bad an idea. You're changing a "this variable will never be reassigned" to "this variable might be reassigned later in this function (probably depending on some condition)". I think the small difficulty added by using const is justified here.

...

... aggressive use of const devalues the operator

The writer suggested usage of const is: Constants should be declared at the top of modules and only in module scope. I don't see how the value of those const declarations decreases by allowing other const declarations inside functions or blocks. I think it's quite easy to distinguish the case of "const declarations which are at the start of a file" from the rest const usages.

...

By const being the default declaration, let rises as the more visible style of declaration. The idea is that let flags where something funny is happening.

I think the premise (let flags where something "funny" is happening) is wrong. It just means what it means, that this variable might be reassigned later in the function, (probably) depending on some condition.

...

However, function arguments like function(a,b,c) { are also allowed re-assignment, so it is a false sense of security to suggest no let means no funny business is happening.

Again, I think no one is saying no let means no funny business is happening in the first place. From the point of view of what JavaScript allows, I think we should consider function arguments to be implicit let declarations, although I think it's bad practice to reassign them (and that can be enforced via style rules). But I don't see how this affects the const vs let discussion.

...

What is “expressed” by const itself when used this way? Since you are intended to refactor the declaration to let if the situation requires it, it can only express “this variable wasn’t being re-assigned when I wrote this code, but feel free to change that”. This is basically meaningless.

I don't think const necessarily expresses "feel free to change that", and "when I wrote this code" is redundant. It just expresses "this variable is not reassigned".

...

Second, choosing const first really means choosing to think about every declaration. Will the next line of code change this assignment? How will I use this variable? Choosing let first eliminates this intellectual load, helping developers focus on more important problems.

As I said before, it's hard to argue against the idea that there is "some" additional intellectual load. My opinion is that this added intellectual load when developing is fairly small, and that it's justified. I also think the examples chosen by the author do not do a good job at showcasing this added intellectual load. It should be quite obvious that a variable storing the array length before a loop should be const, and the variable of the classical for loop a let.

Putting this on the roadmap in the not-too-distant-future. I did not read the articles, but I appreciate the comments. We should think about documenting all of our coding standards (C/C++, Javascript, Java, Kotlin, etc). We can look at tor's for a starting point, as well: https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandards.md https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandardsRust.md

I am generally very supportive of maintaining readable code that is explicit about which variable bindings may change within scope. Self-documenting code is especially advantageous.

Mozilla are not explicit about when const vs. let should be used in their documentation: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#JavaScript_practices

Trac:
Keywords: N/A deleted, TorBrowserTeam202001 added

Replying to sysrqb:

We should think about documenting all of our coding standards (C/C++, Javascript, Java, Kotlin, etc). We can look at tor's for a starting point, as well: https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandards.md https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandardsRust.md

#32544 (moved)