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

Add tests for components in React StrictMode #779

Closed
devongovett opened this issue Jul 18, 2020 · 19 comments · Fixed by #4564
Closed

Add tests for components in React StrictMode #779

devongovett opened this issue Jul 18, 2020 · 19 comments · Fixed by #4564
Labels
enhancement New feature or request strict mode

Comments

@devongovett
Copy link
Member

🙋 Feature Request

We should have tests for all components to sure they work in react StrictMode. This is the default for new apps in create-react-app, so if there's issues, it's likely people will see them. I believe last time I tried I ran into some problems, so it would be good to fix those and have unit tests to ensure it works. This may also require upgrading some libraries (e.g. react-transition-group?).

🧢 Your Company/Team

RSP

@tomsontom
Copy link
Contributor

I think until this is fixed it should be mentionned in the "Get Started" section at https://react-spectrum.adobe.com/react-spectrum/getting-started.html

@mischnic
Copy link
Contributor

This may also require upgrading some libraries (e.g. react-transition-group?).

Updating and moving to this pattern of passing a ref to <Transition> so that it doesn't use findDOMNode anymore: https://github.com/reactjs/react-transition-group/blob/master/CHANGELOG.md#440-2020-05-05

@stefanprobst
Copy link
Contributor

hi, echoing the above - i think it would be good to mention in the docs somewhere that currently there are issues with StrictMode (running into #977 myself).

@peterjcole
Copy link

Sorry for the slightly off-topic question, but hopefully it may be useful for others

I'm seeing the mismatched server/client IDs detailed in #2231 in our Strict Mode application in development

Since Strict Mode only runs in dev, does this mean that shouldn't be something we need to worry about in production?

@LucasUnplugged
Copy link

Okay, thanks @ivanjeremic. Sounds like it's the same issues and workarounds we see here, unless for some reason you CANNOT disable strict mode.

@ivanjeremic
Copy link

I can disable strict mode but in the future I will maybe want to use some of the new react features and if spectrum doesn't catch up I will lock myself out of new react features, are there any infos on it has any work on updating spectrum started yet?

@LucasUnplugged
Copy link

I have no idea, I'm not part of the project team.

But I agree that hopefully this will still be addressed at some point.

@FranCarstens
Copy link

FranCarstens commented Dec 13, 2022

Removing StrictMode fixed this for me; but I'd rather not :( Subscribing to this issue

@devongovett
Copy link
Member Author

We do plan to support StrictMode, and we are working on it. However, this is a large project and it will take some time for us to work through all of the issues. The setup work to enable StrictMode in storybook on a per-story basis is in #3333, and was already added to the tests in #3241. We also have a lint plugin to help find some known issues in #3835.

Once this setup is complete, we will be able to incrementally work toward StrictMode support across the components. Any help or contributions toward this goal would be very much appreciated!

@devongovett
Copy link
Member Author

On a side note, react-transition-group was updated in #3706 which should help with some issues (e.g. legacy context usage). This will go out in our next release.

@cdierkens
Copy link

Work around for id hydration warning.

      <Label {...labelProps} suppressHydrationWarning>
        {label}
      </Label>

@wilsonhou
Copy link

wilsonhou commented Feb 2, 2023

Is this being actively worked on?

hi, echoing the above - i think it would be good to mention in the docs somewhere that currently there are issues with StrictMode (running into #977 myself).

Agree that it should be mentioned in the getting started docs as a gotcha. This (#3515) tripped me up for several hours today in a Next.js app using our DS (that depends on react-aria/react-stately).

@snowystinger
Copy link
Member

Yes, this is actively being worked on and is one of our priorities right now.
See #3878 for some FocusScope fixes
See #3980 for id hydration warning
We can run our test suite in Stictmode now, feel free to run it and fix any that are causing you issues. We accept PRs.

@seanlh
Copy link

seanlh commented Mar 2, 2023

Ran into this issue with a brand new Create-React-App trying to use the Picker after spending some time trying to figure out why I could not get a list to populate.

I strongly second adding an alert on the getting started page indicating that some components may not work in strict mode and that it is being worked on to save others the trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request strict mode
Projects
None yet
Development

Successfully merging a pull request may close this issue.