-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Comments
Thanks for the tip! This should be fixed now :) @all-contributors please add @jaquinocode for bugs |
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
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
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):
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:
initialName
prop set tofoo
."foo"
. Delete all text in input so that it's blank.""
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 keyname
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.
The text was updated successfully, but these errors were encountered: