Skip to content

CPLAT-8038 Implement/Expose useCallback Hook #233

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

Merged
merged 8 commits into from
Dec 2, 2019

Conversation

sydneyjodon-wk
Copy link
Collaborator

@sydneyjodon-wk sydneyjodon-wk commented Nov 18, 2019

Motivation

Support useCallback hook in react-dart

Changes

  • Added useCallback function
  • Wrote tests.

Please review: @kealjones-wk @aaronlademann-wf @greglittlefield-wf @joebingham-wk

@sydneyjodon-wk sydneyjodon-wk changed the base branch from master to CPLAT-8034-usestate-hook November 18, 2019 21:41
Copy link
Collaborator

@kealjones-wk kealjones-wk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+10
I went in circles on this in my head and DID NOT understand what useCallback did it took me FAR too long lol i should have just looked at your tests...

lib/hooks.dart Outdated
Comment on lines 107 to 110
/// Note there are two rules for using Hooks (<https://reactjs.org/docs/hooks-rules.html>):
///
/// * Only call Hooks at the top level.
/// * Only call Hooks from inside a [DartFunctionComponent].
Copy link
Collaborator

@kealjones-wk kealjones-wk Nov 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#nit, we can do this later and not a big deal, and i only just realized this after reviewing all these, that maybe this should be formatted like this:

/// > __Note:__ there are two [rules for using Hooks](https://reactjs.org/docs/hooks-rules.html):
/// >
/// > * Only call Hooks at the top level.
/// > * Only call Hooks from inside a [DartFunctionComponent].

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this in a couple places and I'll make sure the rest are updated in the overall hooks branch

@greglittlefield-wf greglittlefield-wf changed the base branch from CPLAT-8034-usestate-hook to 5.2.0-wip November 21, 2019 21:25
react_test_utils.Simulate.click(incrementDeltaButtonRef);
});

test('callback stays the same if state not in dependency list', () {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice tests here!

@greglittlefield-wf
Copy link
Collaborator

+10

@greglittlefield-wf greglittlefield-wf merged commit 50bd3f6 into 5.2.0-wip Dec 2, 2019
@aaronlademann-wf aaronlademann-wf added this to the 5.4.0 milestone Jan 29, 2020
@greglittlefield-wf greglittlefield-wf deleted the CPLAT-8038-usecallback-hook branch February 16, 2022 21:53
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.

4 participants