Rewrite usage of ReactDOM.render to React 18

[martijnrusschen]

ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it’s running React 17.

Blocking #4207 currently. See https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-client-rendering-apis for guidance on how to use the new render.

[martijnrusschen]

@frankops11 If you're looking for a nice challenge, there are 13 usages across 4 files in the code base. These need to be rewritten to the new React 18 way.

[franmc01]

Challenge accepted!🔍🛠️

[franmc01]

@martijnrusschen accepting the challenge has been quite an adventure. After making changes in 'docs-site' and 'examples', no issues arose; everything went smoothly. However, when we dove into the universe of unit testing, that's where the house came tumbling down, haha! Given that we are using Enzyme and, to this day, there is no support for React 18 nor anything official about it, many tests simply break, and honestly, I don’t know if you have tried to tackle something similar. One strategy that occurred to me was to migrate some of those tests to Testing Library, but I'd love to hear your thoughts on this.

[martijnrusschen]

It looks like the problematic part is the Enzyme adapter that's specifically for React 17. It looks like Enzyme and Jest go well together though: https://enzymejs.github.io/enzyme/docs/guides/jest.html. Can we get rid of the React 17 adapter?

[franmc01]

Thank you for pointing out the issue and providing additional resources 🙏.

I've explored the Enzyme's compatibility thread and the article "Enzyme is dead, now what?" to grasp the current situation with Enzyme, particularly concerning its compatibility with React 17 and the forthcoming versions.

It seems like the path forward with Enzyme is a bit murky 🌫️, especially considering the move towards react-testing-library by many in the community due to the uncertain future of Enzyme. The particular concern with React classes in future versions does pose a valid concern 🚧.

Given the circumstances and for future-proofing our testing architecture, it might be prudent to explore alternative testing utilities that have clear support and roadmap with React's future versions. react-testing-library is a prominent candidate, but we must consider the refactoring costs and any testing paradigm shifts it may introduce 🔄.

I'd love to discuss and strategize on how we can smoothly transition our testing stack to align with the evolving React ecosystem while mitigating risks and ensuring that our testing suites remain robust and effective 🚀.

Looking forward to collaborative thoughts and further discussions!

[martijnrusschen]

That's a good thought. We just moved the test suite from Karma to Jest which has been a major change already. I think that making it further future proof would definitely be a good move to make sure we don't need to change again in the future. We could try to remove Enzyme all together to see how many tests break so that we have an understanding of the impact and effort to make this happen.

Pinging @Zarthus and @polbene95 for some input as well.

[martijnrusschen]

This document outlines how to migrate from Enzyme to RTL. Doesn't seem to be a major challenge looking at the minimal usage we have in the code base: https://testing-library.com/docs/react-testing-library/migrate-from-enzyme/

[martijnrusschen]

@frankops11 Let me know if you want to try removing Enzyme all together.

[franmc01]

Yep, I'm on board with looking into ditching Enzyme. Just curious about the urgency level here - how fast are we wanting this done? I'm all in for contributing but just to set expectations, my time's a bit patchy during the week (full-time job things, you know). So, a big YES from me, and always happy for any help or tips from you all to make things zoom along! 🚗💨

[martijnrusschen]

156 occurrences of Enzyme left.

[martijnrusschen]

cc @yuki0410-dev

[yuki0410-dev]

@martijnrusschen
Thanks for your quick review and merge.
There are now only two more files that I have not refactored. (datepicker_test.test.js, calendar_test.test.js)
They are very long files and may take some time.
After they are done, I will start removing the little bit of Enzyme I have left.

[martijnrusschen]

Thanks for all help! I have been tracking the hits on Enzyme in this pullrequest: #4563. We're almost there!

[yuki0410-dev]

My thought is that after removing all Enzyme tests, that Linter may not be needed if the Enzyme is removed from package.json. (The Enzyme test will not be able to run itself.)

[martijnrusschen]

Agreed!

[martijnrusschen]

Enzyme has been removed completely now. Thanks to @yuki0410-dev!