Skip to content
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

Code snippet in docs unexpectedly throws #173

Closed
shawninder opened this issue Oct 17, 2017 · 4 comments
Closed

Code snippet in docs unexpectedly throws #173

shawninder opened this issue Oct 17, 2017 · 4 comments

Comments

@shawninder
Copy link

Bug Report

The docs present an example code snippet which crashes the js process. See https://reactjs.org/docs/refs-and-the-dom.html#adding-a-ref-to-a-dom-element

In particular, notice the highlighted parts of the code snippet and the first sentence under the snipppet, which reads "React will call the ref callback with the DOM element when the component mounts, and call it with null when it unmounts.".

  1. If the ref callback is called with null, the callback will be doing this.textInput = null;.
  2. From that moment on, if a user clicks the second input (the button), the focusTextInput method will do this.textInput.focus();.
  3. But since this.textInput is null, it's really doing this.null.focus();.
  4. Yet focus is not a method of null, so this will throw

I would suggest adding a little guard for this in the code snippet so that people can copy-paste without worry, replacing this:

this.textInput.focus();

with this:

this.textInput && this.textInput.focus();

or something like this.

This was found in the docs for React v16.0.0

@Ethan-Arrowood
Copy link

@shawninder are you planning on submitting a PR for this?

@bvaughn
Copy link
Contributor

bvaughn commented Oct 17, 2017

I don't think this is actually a problem, is it?

The code in focusTextInput would only run in response to the input onClick handler, which wouldn't be invoked after an unmount (b'c the input would have also been unmounted).

Null checks before accessing refs are a good idea if the code in question might execute before a mount or after an unmount (eg as the result of a timer, promise, or network request) but I don't think it's actually a problem in this case.

Maybe I'm misunderstanding this? 😄

@shawninder
Copy link
Author

Hmm I guess you are right @bvaughn, the problem I mention would only occur if you place the two inputs in different components, allowing the button to be mounted while the other input is not. So no, @Ethan-Arrowood, I will not submit a PR for this.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 17, 2017

No worries at all. I appreciate you bringing this to our attention, just in case. 👍

jhonmike pushed a commit to jhonmike/reactjs.org that referenced this issue Jul 1, 2020
Co-Authored-By: CaiqueMOliveira <caique.m.oliveira.br@gmail.com>

* add missing single quotation on the end of the test description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants