New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Context of getDerivedStateFromProps
is null
#12612
Comments
See my comment in the PR. You cant access static methods on a class via |
Hey @jquense, thanks for your reply! I'm not sure however if we're aligned on the issue here. My intention wasn't to pass the instance as the context to gDSFP but the actual constructor. One should be able to access other static methods from a static method (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/static#From_another_static_method). The usual context within static class methods is thus the constructor (indeed not the instance). However in gDSFP the context is explicitly set to |
oops sorry @timdegroote reviewed to quickly. I'm still not sure whether this should be considered a bug, my guess is the team explicitly chose the I'll open it all back up |
Yeah, at least from my point of view the intention is to highlight that I understand this might seem a bit overprotective. But I don’t see practical upsides to allowing it. And I see downsides: This convention isn’t even a good shorthand convenience. Because if you want to make it shorter you can hoist it in a constant or a module level variable or function. It’s both easier to reference and more explicit. It’s also better encapsulated because other modules can’t access it. |
The way I ran into this was as following: I had a use case for gDSFP and wanted to try it out. After reading the documentation I realised I would need some methods that were initially on the instance to be ran on certain props so I started converting them to static methods (I agree this isn't the best solution, but this was so far just a test anyway). Then I ran into the issue that I couldn't access my static methods from the gDSFP method. I initially thought my understanding of static methods was incorrect. After confirming that this wasn't the case I thought maybe something was wrong with my babel/webpack setup, so I spent some time there making sure the proper plugins etc were installed and configured. It wasn't until I finally had a look into how the gDSFP method was called from React that I understood what the issue was. Just adding this to the conversation to point out that for users that might expect the context to be consistent with the language spec it will also be confusing. If they don't get down to the actual cause of the problem they might suspect something is wrong with their setup or get the wrong idea of how static methods work. In the end I agree with your opinion and thought process. I can definitely see where you're coming from and understand the decision. Thank you for the clarification! |
In JS the context is determined by the callsite so I wouldn’t say it’s “inconsistent with the spec”. I do agree it can be confusing when you’re already familiar with this pattern, or if you’ve read about it while searching for a solution. In my anecdotal experience people coming to JS from other languages tend to first try Here, the confusion is explicit. You know it just didn’t work. You researched the problem, got an answer, and filed an issue. Somebody else will find it later, post it to StackOverflow, and eventually it will become common knowledge. The important part is that you knew there was a problem. The far more unpleasant case is when you don’t know you have a problem. Such as with quietly undefined fields. Or where you can’t identify the problem. It’s easier to identify So I think overall even though both approaches have problems, I’d prefer a problem that “fails fast” over the one that can subtly hide a misunderstanding. Even if the second one might be more conventional in some JS coding styles. |
I don't like this decision. I took me too much time checking why "this" was null. I even started to think that maybe it's just in my head that static methods can call each other since "this" is the context of constructor/prototype. IMHO the proposed change should be merged since this is what javascript developer should expect. The intention to avoid confusion in people coming form other than JS languages in my opinion might lead to more confusion in the future. Also: I know inheritance is discouraged in the community, but some developers use it for some approaches and without proposed change some things might be harder to achieve. |
Actually,i don`t konw how to use getDerivedStateFromProps;the method cannot stop a class life cycle ,even thought it return null. |
Do you want to request a feature or report a bug?
A bug
What is the current behavior?
When
getDerivedStateFromProps
is called the context in which it is executed isnull
, making it impossible to access other statically defined class methods throughthis
.JSFiddle: https://jsfiddle.net/Luktwrdm/390/
What is the expected behavior?
I would expect the provided context to be the class in which the method is defined. Example of a simple class in which other static methods are accessed: https://jsfiddle.net/33nc7jhg/3/.
The text was updated successfully, but these errors were encountered: