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

Bug report (w/ proposed solution): 02 exercise solution shows incorrect text sometimes. #164

Closed
jaquinocode opened this issue Nov 28, 2021 · 2 comments · Fixed by #176
Closed

Comments

@jaquinocode
Copy link

jaquinocode commented Nov 28, 2021

https://github.com/kentcdodds/react-hooks/blob/0b2d92cc8cca98ad5330dd909821805aa6577a9e/src/final/02.js#L8

Above code needs to be changed to below code (same goes for all the other 02 extra credit solution files):

 window.localStorage.getItem('name') ?? initialName,

This is because the current code causes a bug. The bug is basically not properly accounting for the edge-case where .getItem() can return an empty string. So then the input element will show incorrect text. (When this bug happens and how it looks will be clear when you follow the below steps to reproduce.)

Steps to reproduce the bug:

  1. Edit the 02.js solution file so that you give the Greeting element an initialName prop set to foo.
  2. Run 02 solution on web page.
  3. You'll see that input will have text "foo". Delete all text in input so that it's blank.
  4. Refresh the page.
  5. Here you would expect your empty input value of "" to be loaded from local storage instead of "foo". After all, that is the last value you set for the input. The component should remember that and treat the empty string no differently than any other string. "foo" should only be shown if there is no value with key name in local storage. But instead, you'll see that the result is that the input shows the "foo" text.

You might also want to create a test to check for this bug, since the 02 exercise tests don't catch this bug. And the bug is really easy for your students to make and not notice, so it'd be useful to do I think.

@kentcdodds
Copy link
Member

Thanks for the tip! This should be fixed now :)

@all-contributors please add @jaquinocode for bugs

@allcontributors
Copy link
Contributor

@kentcdodds

I've put up a pull request to add @jaquinocode! 🎉

SherylHohman added a commit to SherylHohman/kcdreact-2-react-hooks that referenced this issue Feb 19, 2022
src/final/02.extra-3.js presents the same behaviour described in issue epicweb-dev#164.
Commit fdfe357 fixed the issue in related files. Same fix is now applied here.

fixes epicweb-dev#164
kentcdodds pushed a commit that referenced this issue Feb 19, 2022
src/final/02.extra-3.js presents the same behaviour described in issue #164.
Commit fdfe357 fixed the issue in related files. Same fix is now applied here.

fixes #164
SherylHohman added a commit to SherylHohman/kcdreact-2-react-hooks that referenced this issue Mar 24, 2022
src/final/02.extra-3.js exibited the same behaviour described in issue epicweb-dev#164.
Commit fdfe357 fixed the issue in related files, but not this file.

fixes epicweb-dev#164
paulocuambe pushed a commit to paulocuambe/react-hooks that referenced this issue Sep 9, 2022
paulocuambe pushed a commit to paulocuambe/react-hooks that referenced this issue Sep 9, 2022
…b-dev#176)

src/final/02.extra-3.js presents the same behaviour described in issue epicweb-dev#164.
Commit fdfe357 fixed the issue in related files. Same fix is now applied here.

fixes epicweb-dev#164
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

Successfully merging a pull request may close this issue.

2 participants