Why are underscore prefixes for internal methods of a React component considered bad?

The third bullet under react#methods says:

Do not use underscore prefix for internal methods of a React component.

I was just wondering what the reasoning behind this was?

They give people a false sense of "private"ness that could lead to bugs.

@lencioni

Okay. Even though JavaScript doesn't support private variables, I still think the underscores help communicate the programmer's intent. If possible, @lencioni could you give an example of this false sense of "private"ness that lead to a bug? Or maybe a scenario where the underscores would confuse another programmer and lead them astray. I'm not disagreeing with you, I just want to have a better understanding of how this rule came about.

Private means inaccessible. Your intent to privacy is irrelevant if the value is reachable, ie public.

For example, npm broke node once by removing an underscore-prefixed variable.

npm broke node once by removing an underscore-prefixed variable

If the variable wasn't prefixed they would have used it either way, and node would have still broke. When a meant-to-be-private variable is prefixed with an underscore there is at least one line of defence (a weak one, granted) preventing its use. Whereas when a meant-to-be-private variable isn't prefixed with an underscore, there is no line of defence (not even a weak one).

@ttmarek

@ttmarek yes, but had they not prefixed it with an underscore, nobody would have been fooled into thinking it was actually private, and there would have been test coverage, and changing it would have prompted a semver-major version bump.

An underscore is in no way, whatsoever, a line of defense - it's just an incorrect assumption some developers make about a language that simply does not have the concept of privacy outside of closures.

@ljharb

@ljharb So, in other words: underscore prefixed methods might confuse programmers from other languages into thinking that JavaScript supports private methods. Right?

That, or programmers who think that convention is the same as privacy.

@ljharb

Okay, cool. Thanks @ljharb, I see what you're getting at. Do you think its worth adding a short blurb about this under the third bullet in react#methods?

I think so. I'd be happy to merge a PR that does that.

Sure, more helpful info is always good.

My favorite reason:

Private stuff tends to change without warning, and without advertising as a breaking change.

The problem with that is that a lot of code could break without warning, because EVERYBODY ignores underscores:

Newbies don't know what it means. Experienced developers think they know what they're doing and the underscore doesn't apply to them.

Admit it. You've done that, too.

@lencioni

I think @lencioni is perfectly correct by saying

They give people a false sense of "private"ness that could lead to bugs.

... but I'd like to elaborate on why.

In programming languages in general, the point of having a private property (as opposed to having all properties public) is so that you can trust a private variable not to be changed by outside code, unlike public properties which you need to assume can be changed by outside code at any time.

If you're absolutely hell bent on using inheritance in JavaScript, you don't have private properties. It is not a feature of inheritance in JavaScript. Everything is public. The logical thing to do is therefore to treat every property as a public property (because it is) - your code needs to assume that it can change at any time.

You can call a property _myProperty or __DONTFUCKINGCHANGEmyProperty but that doesn't make that property private. It's public. Private does not exist in the object composition model that you have chosen. You're trying to reap the benefits of a feature that you do not have. If you write your code in a way that assumes that your public-property-with-special-name-convetion does not change, you're just fooling yourself. The fucker can change at any moment, just like any other of your properties.

(Sidenote: This is React, and the following is not an option there, but in general, unless you need to create more than 50000 items per second, I personally think that you should consider using factory functions (https://www.youtube.com/watch?v=ImwrezYhw4w) instead of inheritance for creating objects. They have real privacy and do not have this problem)

Hi everyone, I've landed here after searching for an ESLint rule for enforcing this convention. Since I haven't found any, I've written one myself. In case anyone is interested, here it is: https://github.com/buildo/eslint-plugin-method-names

@ljharb

@ljharb You have a very peculiar definition of impact, but I agree that linting is intend to improve code, which effectively impacts it.

Again, running untrusted code does not erase "all bets" for secrecy and robustness - it only erases a large category of them. The only thing that makes "all bets off" is if untrusted code runs before your code, at which point, essentially nothing can be secured.

That is a pretty bold and risky proposition, and unless the same guide forces one to freeze builtins, useArray.$method and Object.$method instead of someArray.$method, someObj.$method et al, and a whole heap of other concerns, then the argument that using closure would provide you secrecy is pretty short sighted.

Even then if you take all the necessary precautions and measures, running untrusted code in the same context that you have secrets is foolishly naïve. It is that simple.

But then again, that is all irrelevant. I must iterate that Encapsulation and Visibility -- and by extension the use of underscore as convention to denote privacy -- is not a security concern, it is for separation of concern and differentiating implementation from the interface.

The interface contains every observable aspect of it. If _foo is observably present, it's part of the interface, no matter how much your documentation may try to insist otherwise.

Interfaces are protocol and contracts and like any other, they come with requirement and stipulations, if one opts to ignore the warning and recommendation of the API then they're on their own; if one does dumb things they end up in stupid situations.

@ericelliott

Guys, I am sorry but I think this is obvious for everyone that this is not going anywhere and is useful for neither the participants nor the readers.
I was quite happy to see @ericelliott providing some data. I wish we could go in a path a bit more scientific like this. I am a bit sad it was not really followed on.
Without data (and I think you guys have a lot of this), this discussion is about opinions and ideas. I think we clearly reached the end of it and are currently locked inside a bad while(true) argue().
I respect opinions (in particular from experienced people). But I trust data. Properly analyzed, that produces knowledge.

hmm.. As I have understood it, this is basically a discussion about whether or not underscores should be allowed for method names. I kind of agree with the whole discussion about it not being private etc. I have however seen ALOT of React developers using underscore for states which are only UI states ex. the state "collapsed" in a menu component. This is something that I thought made sense and was doing myself until I started using this config and got a ton eslint errors. Up to this day, I still think this a good use of underscores and makes the code easier to reason about. But this is an aspect that not very many people seem to agree with or put a thought too over here.

Just wanted to add my 5 cents to the discussion.

Google MDC uses underscore as a suffix.
Facebook-relay uses underscore as a prefix.
And Airbnb uses no underscore.

I experimented with all approaches.

"No private" is not an option for me, b/c it makes hard to reason about the class, and I'd like a rule that every public method should be documented. It forces you to think about your interface. I don't care about real data privateness, but I never understand sentences like "intractable people problem", b/c undesired underscores access easily can be spotted on a code review unlike variables without any mark.
Underscore as suffix works badly with my eyesight. I constantly miss them, and can't spot an error on a quick look over a codebase.
So I choose to use underscore prefixes for private methods. And just consider event handlers are part of public class API (b/c they called outside), so no underscores for them.

Once class instance fields reach stage 4, you won’t have to compromise on it.

Once class instance fields reach stage 4, then I'll just find and replace my underscore prefixes ;-).

@shelacek

@shelacek and then that will be a semver-major/breaking change for each file in question. As long as you understand that, then go for it.

@ljharb

@ljharb Yeah, this is the core of this discussion. We have 2 groups of people here. For one, this will be change in the revision, because there is virtually no change in API. For the other, this will be a major change simply because it does not recognize this convention. Nevertheless, thank you for your opinion.

Private class field is now supported by some runtime.
Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields

If you're stuck in the runtime that doesn't support private class field, and you want to prevent class consumer to access the private fields and methods, below are some workaround:

hide private methods with local functions
hide private fields with closure or local hashmap

Example for javascript runtime that doesn't support private field natively:

person.js:

let persons = new Map()

// "private method"
function getAge(self) {
  return persons.get(self).age
}

class Person {
  constructor(name, age) {
    // public field
    this.name = name
    // "private field"
    persons.set(this, { age }) // private field
  }

  // public method
  speak() {
    // calling "private method"
    console.log('inside:', this.name, getAge(this))
  }
}
module.exports = Person

consumer.js:

let Person = require('./person')
let alice = new Person('Alice', 18)
alice.speak()
console.log('outside:', alice.name, alice.age)

console log:

inside: Alice 18
outside: Alice undefined

You'd want to use a WeakMap, not a Map, to avoid a memory leak, but yes, private class fields are a 100% safe and ideal approach for something you might otherwise stick an underscore on.

Navigation Menu

Repository navigation

Why are underscore prefixes for internal methods of a React component considered bad? #1024

93 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Why are underscore prefixes for internal methods of a React component considered bad? #1024

Description

Activity

lencioni commented on Aug 19, 2016

ttmarek commented on Aug 19, 2016

ljharb commented on Aug 19, 2016

ttmarek commented on Aug 19, 2016

ljharb commented on Aug 19, 2016

ttmarek commented on Aug 20, 2016

ljharb commented on Aug 20, 2016

ttmarek commented on Aug 20, 2016

lencioni commented on Aug 20, 2016

ljharb commented on Aug 20, 2016

ericelliott commented on Aug 26, 2016

mpj commented on Aug 26, 2016

gabro commented on Sep 5, 2016

93 remaining items

omeid commented on Jun 15, 2017

QuentinRoy commented on Jun 15, 2017

JohanG2012 commented on Jun 24, 2017

kmmbvnr commented on Feb 16, 2018

ljharb commented on Feb 16, 2018

shelacek commented on Jul 3, 2018

ljharb commented on Jul 3, 2018

shelacek commented on Jul 3, 2018

omeid commented on Jul 4, 2018

ljharb commented on Jul 4, 2018

omeid commented on Jul 4, 2018

ljharb commented on Jul 4, 2018

beenotung commented on Sep 15, 2020

ljharb commented on Sep 15, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions