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 support checklist #2011

Open
chenesan opened this issue Feb 8, 2019 · 39 comments

Comments

@chenesan
Copy link
Contributor

commented Feb 8, 2019

Sorry for removing issue template. Since the React 16.8 is out we can start to work on hooks support in enzyme now. Previously I tried to open PR in #2008 to help with this but found out that there are too many apis in React hooks. After some thinking I feel it's better to track all of the supports / PR in one issues, and create one PR for each hooks api (useState, useEffect, etc).

known issues (some are resolved with newer version of react):

  • setState returned from useState not works with shallow before react@16.8.5 . Fixed in facebook/react#15120 .
  • useEffect and useLayoutEffect not works with shallow because react shallow renderer doesn't run it. (facebook/react#15275)
  • useCallback doesn't memoize callback in react shallow renderer (facebook/react#15774)

Currently I'm working in useState (#2008) and I'm glad to see if anyone else would like to help to support other apis. For now we need to add many tests to make sure what is working in enzyme, and what is not, then add patch / feature into it. I can help to maintain the above checklist if there are any related issues / PR .

I'm simply volunteer to do this and not a main maintainer here though so @ljharb if you have had any plan / schedule on this feel free to tell me and I can change or close this :-)

@k3ithl1m

This comment has been minimized.

Copy link
Contributor

commented Mar 3, 2019

Thanks for making this list @chenesan!
@ljharb, I'm interested in tackling some of this features too. It might take me some time to finish implementing it as I'm getting a little busy in the next two weeks, but I am very interested in helping out.
I've currently worked on two test cases #2029

If no one has took up the useEffect feature. I would like to try that out!

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Hi @k3ithl1m glad to see your work :) FYI I just open #2034 to wrap render function of mount renderer with act(). That should be related to useEffect test cases and I hope that help!

@venikx

This comment has been minimized.

Copy link

commented Mar 7, 2019

@chenesan Also note that React seemed to have an issue with shallow rendering hooks right now. Not sure if it was addressed by the React time already or not.

@nixstrom

This comment has been minimized.

Copy link

commented Mar 7, 2019

@AnaRobynn Didn't they solve this with facebook/react#14567 ?

@venikx

This comment has been minimized.

Copy link

commented Mar 7, 2019

@nixstrom It’s supported, but there are issues apparantly. I saw Dan tweeting about it yesterday: https://twitter.com/dan_abramov/status/1103304963445403648?s=21

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

I think the issue should be facebook/#14841, which #2008 would depends on.

@pgangwani

This comment has been minimized.

Copy link

commented Mar 9, 2019

@chenesan @k3ithl1m : Added test cases #2041 for useEffect, useState & custom hooks along with combination of those. Will add few more soon. I was thinking to get some support as a focus (micro) team for react hooks test cases considering all scnerios.

@pgangwani

This comment has been minimized.

Copy link

commented Mar 10, 2019

@chenesan : Please add #2041 in custom hooks checklist item as I added test case in that PR

@pgangwani

This comment has been minimized.

Copy link

commented Mar 10, 2019

@ljharb @chenesan : Simple hooks testing is successful in mount rendering without act :)
Example: https://codesandbox.io/s/5xkjk1ql14

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

@pgangwani I believe the test could pass without act. However when it comes to setting state in useEffect without act wrapping and assert the state changes after mount (like test case of #2034 ), it would fail. Since #2034 is merged this should be fine to mount render without act(), though.

@pgangwani

This comment has been minimized.

Copy link

commented Mar 11, 2019

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

I'm not sure if that would pass without act wrap. but since we wrap mount with act in master now, this should be fine I think.

@pgangwani

This comment has been minimized.

Copy link

commented Mar 11, 2019

@pgangwani

This comment has been minimized.

Copy link

commented Mar 18, 2019

@chenesan : [Update]: Added test cases for useReducer & useContext. Few custom hooks too

@tlginn

This comment has been minimized.

Copy link

commented Apr 11, 2019

Are there any updates on when useMemo will be supported with shallow rendering?

Specifically I'm rendering a connected react-redux component. Since react-redux is now relying on hooks, I'm getting the "Invalid hook call" when it attempts to call useMemo.

I can workaround using mount for now, but would prefer to keep all of my tests using shallow.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Apr 12, 2019

@tlginn in the next release of enzyme, hopefully.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 4, 2019

v3.10.0 has now been released.

@yuritoledo

This comment has been minimized.

Copy link

commented Jun 4, 2019

Are there any changelog or docs? @ljharb

@vydimitrov

This comment has been minimized.

Copy link

commented Jun 4, 2019

Yes, here. But seems nothing hooks related.

@yuritoledo

This comment has been minimized.

Copy link

commented Jun 4, 2019

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2019

Most of the hook-related things are in the react 16 adapter.

@Hypnosphi

This comment has been minimized.

Copy link

commented Jun 5, 2019

Most of the hook-related things are in the react 16 adapter.

Which doesn't seem to have a changelog at all =( https://github.com/airbnb/enzyme/tree/master/packages/enzyme-adapter-react-16

@MattyBalaam

This comment has been minimized.

Copy link

commented Jun 5, 2019

I have had a little play and I’ve been able to remove act() in certain situations in tests with components using hooks. But I still need to use it when using useState

@tarikhamilton

This comment has been minimized.

Copy link

commented Jun 5, 2019

Is the checklist at the top updated, @chenesan ? I'm confused by some of the comments that follow. I can't tell if hooks are supported in Enzyme yet.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2019

@Hypnosphi the changelog for the adapters is in the version bump commit itself: dc724f0

@travi

This comment has been minimized.

Copy link

commented Jun 5, 2019

i will admit that i've had a hard time understanding what changes are included in which package as well.

i've found that to be true in almost all monorepos, though, so its not in any way limited to this one (part of the reason i think monorepos favor maintainers over consumers, which is probably the appropriate balance in many ways).

i usually look for a github release first, then try the changelog, but often forget to check the commit (or lose motivation by that point).

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2019

I hate monorepos, but it's what makes the most sense for this project.

I would be more than happy to (in a separate issue or PR) discuss suggested alternatives for changelog management that makes things more discoverable and consistent.

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

@tarikhamilton I tried updating related issues and test case pr after a perod of time. The known issues are recorded in the issue body. Maybe there are still some use cases we didn't cover(but we don't know), if you find any issue please feel free to open an issue or comment so we can support hooks more properly.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 8, 2019

@chenesan it'd be helpful if you could check off the boxes in the OP that are completed.

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

@ljharb I just check off the item whose test cases are merged; Most of the items are stuck at #2041 .

@bencoullie

This comment has been minimized.

Copy link

commented Jun 9, 2019

Thanks for the PRs and work here. What is the best way to know when we can use hooks like useEffect in our components that are tested by Enzyme's shallow render etc? Is it to check in on this issue and it's checklist periodically? Will it be in a discoverable changelog? Or should I watch facebook/react#15275?

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

@bencoullie You could watch facebook/react#15275 and facebook/react#15589. Not sure if it is possible to fix this with only works in enzyme side .

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 11, 2019

I've merged all the outstanding hooks-related PRs.

Let's put up the PRs needed to close this issue:

  1. document in the readme, known limitations with hooks, and link back to all the filed react/shallow renderer bugs
  2. put up test cases for any missing hook coverage
  3. file followup issues to investigate any skipped test cases that aren't blocked on an upstream FB change.

Thanks!

@chenesan

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

@ljharb Filed an issue for (3) in #2165 . I think there's no other .shallow issue in enzyme side now. Also open #2164 for (1). After these works finished I think we can close this issue :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.