react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

Add tests for components in React StrictMode

Open devongovett opened this issue 5 years ago • 4 comments

🙋 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

devongovett avatar Jul 18 '20 02:07 devongovett

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

tomsontom avatar Sep 15 '20 20:09 tomsontom

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

mischnic avatar Sep 29 '20 10:09 mischnic

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 https://github.com/adobe/react-spectrum/issues/977 myself).

stefanprobst avatar Feb 16 '22 21:02 stefanprobst

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?

peterjcole avatar Aug 18 '22 09:08 peterjcole

Additional Numberfield issue with StrictMode reported on this thread https://github.com/adobe/react-spectrum/issues/2870#issuecomment-1301626183

snowystinger avatar Nov 03 '22 05:11 snowystinger

I understand this might be a big undertaking to fix and I respect that it might take time. This issue is important to me and I would appreciate some assurance that fixing this in the near future is in Adobe's interest. This has been open for over 2 years and I just want to know whether this is being worked on/researched :)

VapidLinus avatar Dec 09 '22 07:12 VapidLinus

So that means I cannot use thi

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

This comment is 2 years old but I agree, I'm on react 18 and after days of working with spectrum I just found out I cannot use it because it uses some deprecated context api and all the work was waste of time mostly. What are the best workaround for this context issue?

ivanjeremic avatar Dec 09 '22 09:12 ivanjeremic

I'm on react 18 and after days of working with spectrum I just found out I cannot use it because it uses some deprecated context api

Do you have any details on this, @ivanjeremic? Has this been reported?

LucasUnplugged avatar Dec 09 '22 09:12 LucasUnplugged

I'm on react 18 and after days of working with spectrum I just found out I cannot use it because it uses some deprecated context api

Do you have any details on this, @ivanjeremic? Has this been reported?

https://github.com/adobe/react-spectrum/issues/3269 Same for the Dialogs.

ivanjeremic avatar Dec 09 '22 09:12 ivanjeremic

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

LucasUnplugged avatar Dec 09 '22 09:12 LucasUnplugged

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?

ivanjeremic avatar Dec 09 '22 10:12 ivanjeremic

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.

LucasUnplugged avatar Dec 09 '22 10:12 LucasUnplugged

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

FranCarstens avatar Dec 13 '22 16:12 FranCarstens

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 avatar Dec 15 '22 21:12 devongovett

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.

devongovett avatar Dec 15 '22 21:12 devongovett

Work around for id hydration warning.

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

cdierkens avatar Jan 31 '23 04:01 cdierkens

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 (https://github.com/adobe/react-spectrum/issues/3515) tripped me up for several hours today in a Next.js app using our DS (that depends on react-aria/react-stately).

wilsonhou avatar Feb 02 '23 03:02 wilsonhou

Yes, this is actively being worked on and is one of our priorities right now. See https://github.com/adobe/react-spectrum/pull/3878 for some FocusScope fixes See https://github.com/adobe/react-spectrum/pull/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.

snowystinger avatar Feb 02 '23 06:02 snowystinger

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.

seanlh avatar Mar 02 '23 00:03 seanlh