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

useEffect for the POST #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

VinSpee
Copy link

@VinSpee VinSpee commented Jun 13, 2019

any side-effects should happen in a useEffect hook. This commit does that by creating a new hook which does the posting to a server, resetting of the todo input, and focusing of the inputEl.

The dependencies array specifies that whenever the isSaving state changes, this effect should execute. That's only half of the story here.

We additionally check inside of the hook that the value of isSaving is true. Then and only then do we go ahead with the post. You'll also notice that I'm not specifying newTodo in the dependency array. That's because I don't need this effect to run every time the value is updated, only when it's submitted.

any side-effects should happen in a `useEffect` hook. This commit does
that by creating a new hook which does the posting to a server,
resetting of the todo input, and focusing of the `inputEl`. The
[dependencies
array](https://reactjs.org/docs/hooks-reference.html#conditionally-firing-an-effect)
specifies that whenever the `isSaving` state changes, this effect should
execute. That's only half of the story here. We additionally check
_inside_ of the hook that the value of `isSaving` is `true`. Then and
only then do we go ahead with the post. You'll also notice that I'm not
specifying `newTodo` in the dependency array. That's because I don't
need this effect to run every time the value is updated, only when it's
submitted.
@samselikoff
Copy link
Collaborator

still not convinced dawg 😄

In all honesty this reminds me of observers from Ember. The one-way flow from event to action still feels safer to me...

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 this pull request may close these issues.

2 participants