wordpress-seo icon indicating copy to clipboard operation
wordpress-seo copied to clipboard

WP 6.2 compatibility: use React `createRoot`

Open igorschoester opened this issue 2 years ago • 10 comments

⚠️ Since we rely on @wordpress/element to export createRoot -- this issue can not be fixed until WP 6.2 is the last version we support (unless we try both methods) ⚠️

In WP 6.2 we get React 18.0.2 instead of 17.0.2.

This causes JS console errors to be shown:

Warning: 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. Learn more: https://reactjs.org/link/switch-to-createroot

This seems to originate in a couple of different places, like the help scout beacon, first time configuration and the settings (not Settings UI).

  • Please look for render itself in our JS source code, as most likely this is anywhere we add a React root.
  • Please do not forget our add-ons!

Reproduce steps

  • Install and activate the WordPress Beta Tester plugin
  • Go to Tools > Beta testing
  • Change the update channel to Bleeding edge and the stream to Beta/RC Only
  • Go to Dashboard > Updates and update to 6.2-beta1 (at the time of writing)
  • Add/ensure the following constants are set as follows in your wp-config.php. Please do this instead of the WP_DEBUG set to false.
if ( ! defined( 'WP_DEBUG' ) ) {
    define( 'WP_DEBUG', true );
}
if ( ! defined( 'SCRIPT_DEBUG' ) ) {
    define( 'SCRIPT_DEBUG', true );
}
if ( ! defined( 'YOAST_ENVIRONMENT' ) ) {
    define( 'YOAST_ENVIRONMENT', 'development' );
}
  • Go to Yoast SEO > General with the console open
  • Verify you see errors like the above

igorschoester avatar Feb 13 '23 14:02 igorschoester

Situation

  • React is loaded via @wordpress/element
  • Version of @wordpress/element is depending on the WordPress version
  • We need to support multiple versions
    • Upcoming is WP 6.2 where WP element goes to 5.3.1 and React to 18.2

Dependencies (TODO)

  • WP element is also a peer dependency in the UI library
    • We decided to keep the same WP based React here for “simplicity”. Is that still something we agree with?
  • React and React DOM are development dependencies for the Storybook
    • We have to keep these inline with WP element’ versions

Tests are failing (TODO)

  • Created https://github.com/Yoast/wordpress-seo/issues/20007
  • Errors on React things, which are probably due to our test suite being multiple things and outdated
    • Using Enzyme, which won’t be able to work with React 18: https://dev.to/wojtekmaj/enzyme-is-dead-now-what-ekl
    • Sometimes using react-test-renderer, version 16
  • It is not straight forward to just upgrade here
  • Dependencies in packages might conflict with those in the root. Could investigate nohoist here

Possible solution?

  • Skip the failing tests
  • Start project to investigate/upgrade our JS testing setup to something more modern
    • https://testing-library.com/docs/react-testing-library/migrate-from-enzyme

Addons (TODO)

  • Same checks need to be done there still

igorschoester avatar Feb 15 '23 09:02 igorschoester

Moved to the most future sprint we currently have, we'll reprise it by then and try to plan a clear timeline

enricobattocchi avatar Mar 16 '23 10:03 enricobattocchi

Please inform the customer of conversation # 1054366 when this conversation has been closed.

shabnam611 avatar Sep 11 '23 08:09 shabnam611

Issue is still active with latest yoast premium. Any ETA ?

pixeline avatar Nov 21 '23 16:11 pixeline

Please inform the customer of conversation # 1103708 when this conversation has been closed.

ogodoabiola avatar Feb 09 '24 06:02 ogodoabiola

Was googling for something else and ended up here. If I am reading this correctly I have been supporting both versions like so:

/**
 * External dependencies
 */
import { createRoot, render } from '@wordpress/element';
import App from './app';

const target = document.getElementById( 'my-app-root' );

if ( target ) {

    if ( createRoot ) {
        createRoot( target ).render( <App /> );
    } else {
        render( <App />, target );
    }
}

If createRoot() exists (like in the newer react) it uses that. If createRoot() is undefined... it falls back to render()

helgatheviking avatar Feb 14 '24 14:02 helgatheviking

Hi @helgatheviking

Thanks for the comment and for sharing more details. This issue is on the radar of our development team, and they will prioritize it based on the severity of this problem in relation to other open bug reports and new features.

josevarghese avatar Feb 15 '24 12:02 josevarghese

Still not corrected?

neoseeyou avatar Jun 14 '24 08:06 neoseeyou