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

React hooks test case part 3 -- useCallback #2148

Merged
merged 1 commit into from
May 31, 2019

Conversation

chenesan
Copy link
Contributor

The tests are almost as same as #2144 , but for useCallback.

And I saw that shallow renderer doesn't memoize the callback between two rendering even the dependencies not change. So the test in chenesan@5cc14ba#diff-2e9c9c89f001140a7699819fc2f5d926R1252 will fail and I temporarily skip it. Have opened an issue in react (facebook/react#15774) . If it's confirmed as a bug of shallow renderer then I'll try to fix it in react side.

`shallow`: Note that the `get same callback when using `useCallback` and rerender with same prop in dependencies` is skipped because react shallow renderer doesn't memoize callback function value. Pending facebook/react#15774
@chenesan chenesan mentioned this pull request May 30, 2019
14 tasks
@ljharb ljharb added the Tests label May 30, 2019
@ljharb ljharb added this to v16.8+: Hooks in React 16 May 30, 2019
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Since facebook/react#15774 is a bug, it'd be good to find a way to work around it before the bugfix is available.

@ljharb ljharb force-pushed the usecallback-test branch 2 times, most recently from f7897d4 to 9cc5f44 Compare May 31, 2019 06:22
@ljharb ljharb merged commit 9cc5f44 into enzymejs:master May 31, 2019
@chenesan
Copy link
Contributor Author

chenesan commented Jun 8, 2019

I just sent a PR for this in facebook/react#15843 ; As for bugfix in enzyme side, I'm not sure what the way is -- inherit ReactShallowRenderer class and override _createDispatcher in https://github.com/facebook/react/blob/master/packages/react-test-renderer/src/ReactShallowRenderer.js#L230 ? That sounds like a big change... but seems a way to fix hooks issue in shallow renderer (for example, useEffect won't work in shallow).

@ljharb
Copy link
Member

ljharb commented Jun 8, 2019

We could stub useCallback in the shallow renderer, perhaps?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
React 16
  
v16.8+: Hooks
Development

Successfully merging this pull request may close these issues.

None yet

2 participants