React: this is undefined in getDerivedStateFromProps

[sanbornhilland]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
this is undefined in getDerivedStateFromProps

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

class MyComponent extends Component {
  static sayHello() {
     console.log('Hello')
  }

  static getDerivedStateFromProps() {
     this.sayHello()
  }
}

What is the expected behavior?
I would expect the above to work. To me it is unexpected that a static method is not getting called on the class.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
16.5.1

This seems like an easy fix to me, here

Is there a reason not to change line 157 and 161?

// From this
getDerivedStateFromProps.(nextProps, prevState);

// To this?
getDerivedStateFromProps.call(ctor, nextProps, prevState);

[TrySound]

@sanbornhilland This is expected. https://github.com/facebook/react/issues/12612#issuecomment-381332338

[milesj]

To me it is unexpected that a static method is not getting called on the class.

Outside of what @TrySound has linked, this is how JS works. Static methods do not have access to a class instance's this, and IMO, it never should.

[sanbornhilland]

It's not even null though, which would indicate that it was intentionally set. It's undefined which looks like a bug to me.

I understand wanting to avoid unnecessary confusion in the React api but this misses the mark IMO. To me this is unexpected behaviour and at some point it seems reasonable to encourage developers to learn how this works instead of being overly protective.

[jquense]

this is intended, and yes it does differ from how static methods, work _if_ you call them with the class to the left of the dot Foo.bar(). For the reasons noted in the other issue The react team has opted to avoid ambiguity and always make sure it's undefined

[sanbornhilland]

@milesj I'm not sure what you mean by

Static methods do not have access to a class instance's this, and IMO, it never should.

That's demonstrably false so maybe I'm misunderstanding your point.

[milesj]

@sanbornhilland That's how JS works.

[milesj]

You can see their usage here: https://github.com/facebook/react/blob/a129259ad6a61790fe1912a87cb45018d942f90c/packages/react-reconciler/src/ReactFiberClassComponent.js#L789

They are simply calling it as a reference. No bind/call/apply, etc. This is standard JS classes.

[sanbornhilland]

@milesj Sorry mate, don't want to turn this into a debate but you're misreading your console output. Your example code demonstrates that this is defined in that case because you are logging function Foo(). The undefined you are seeing is the return value of console.log.

Appreciate the link to the code but that just proves the point I was making. When you assign the function like that then you lose the context of ctor which is why I was advocating for using call, to avoid losing that context.

EDIT: Just to be clear. I think I understand the confusion now. Perhaps you thought I was asking for this to be bound to an instance. Of course that doesn't make sense. As per the original issue, I was asking that static methods have this referencing the class.

Anyway, I see that this is a deliberate decision by the React team so that's fine. I appreciate the quick response, all.

[jquense]

this in a static method can be the constructor (not the instance)

class Foo { static bar() { console.log(this.baz) } static baz(){} }

[milesj]

@sanbornhilland Correct, I was assuming you wanted the instance, not the constructor. Definitely clears up the confusion.

[mizukami234]

This works (not recommended though):

static get getDerivedStateFromProps() {
  return (props, state) => {
    console.log("Hello this is ", this);
  };
}

[dolsem]

This issue should be reopened because this is expected to point to the class instance. See this contrived example. TypeScript typechecker is fine with this code because it expects this to be defined. This code compiles but fails at runtime. This becomes more of an issue when you create a component intended to be subclassable and so you don't know the exact context beforehand, like in the example above. I have to work around it with a getter like @mizukami234 suggested.

  static get getDerivedStateFromProps() {
    return this._getDerivedStateFromProps.bind(this);
  }