Update @wordpress/data to version 4.27.3

[techanvil]

Note to potential engineer who wants to work on this, please see this comment first.

Feature Description

Following on from the update to @wordpress/data 4.23.0 in #1769, we should update to version 4.27.3. This is the latest version that supports (via @wordpress/element) React 16.

This will position us well for an update to React 17, via updating to more recent versions of @wordpress/data and @wordpress/element.

Note that the update to 4.27.3 should be a lot easier than the update to 4.23.0. Version 4.23.0 could quite reasonably have been a major version bump as it made a significant change to the way resolvers work. In contrast the changes from 4.23.0 -> 4.27.3 appear to be much less impactful. Also, no update to versions of @wordpress/element or React will be needed, whereas these versions were bumped via the update to 4.23.0.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• @wordpress/data should be updated to version 4.27.3.
• No JavaScript errors and no new warnings should be triggered when visiting any Site Kit UI.

Implementation Brief

• Install @wordpress/[email protected].
• Search-replace __experimentalResolveSelect to resolveSelect.
• Verify unit tests are passing.
• Verify e2e tests are passing.
• Verify not warning or errors occur.

Test Coverage

• Coverage should not change.

QA Brief

Changelog entry

[eugene-manuilov]

IB ✔️

[kuasha420]

An update on this

While working on the issue we have discovered that the behavior with useSelect hook has changed somewhere along the line from 4.23.0 -> 4.27.3. In the new behavior, only the stores that are select-ed in the initial render inside useSelect will be subscribed for future re-renders. See here.

This is manageable within useSelect callbacks, as the select calls can be made unconditional e.g. by following the pattern of destructuring selectors at the top of the callback.

However it remains a problem for us since any conditionally invoked createRegistrySelector selectors or resolvers will only select on their required stores once invoked. So, within the context of a useSelect, any store that is only referenced within a selector/resolver that is not invoked in the first render will not be subscribed to, and changes to that store won't result in a re-render.

In our code base, we return early in various useSelect and createRegistrySelector callbacks. ie. here, here and here are few examples of this.

This is not easy to find across code base and requires tedious search and refactoring, not to mention testing the changes are correct after refactoring.

This behavior is not limited to the specific version, the latest version of @wordpress/data (8.2.0 at the time of writing) also exhibits the same behavior. There is an issue for it and a PR that resolves this.

However, even if we wait for that PR to be merged it will mean a huge upgrade from 4.23.0 to 8+ because there will be a lot of documented (and undocumented, judging by this very problem) breakage and we'll have to upgrade the supporting libraries and react/@wordpress/element.

A decision is needed here, specially how to move forward. This is unfeasible to upgrade to 8+ in one go. We should either fork it or use patch-package to restore current behavior to keep the path open for our current approach of incremental update, since there's a good chance that the aforementioned PR will be merged and we will not have to search and refactor our useSelect and createRegistrySelector-s.

Huge thanks to @techanvil for helping me and doing the bulk of the investigations into this. ❤️

Relevant slack thread: https://10up.slack.com/archives/CBKKQEBR9/p1675071035696649

cc @aaemnnosttv @tofumatt @eugene-manuilov @felixarntz

[kuasha420]

After further discussion with @aaemnnosttv, I am putting this back to IB. We need to investigate more on how to proceed here. Since it's not super urgent at the moment, we should try to come up with a robust solution to the problem exhibited instead of a fast workaround. Cheers.

Here's the work put into it so far in terms of code. Rest of the time was spent on debugging and investigation by @techanvil and me.

cc @bethanylang @felixarntz

[mxbclang]

Thanks @kuasha420! Since this is going back through definition now, I've removed from the current sprint.

[zutigrm]

The change that is fixing the issue in previous comment, is shipped in version 9.0.0 (first release date after the fix was merged on Mar 29), which has react 18 as dependency. And since there is already an issue for upgrading react #6357 , I investigated the @wordpress/data version jump to 9.0.0. There were several breaking changes along the way, one of them is with usage of registry.register, etc. Also this includes bumping the version of several other packages to meet the dependencies.

After bouncing through errors, js test erros and warnings, and package dependencies list, these are the packages versions that I tested with:

Details

-               "@material/list": "^2.3.0",
-               "@material/menu": "^2.3.0",
-               "@material/radio": "^2.3.0",
+               "@material/list": "^9.0.0",
+               "@material/menu": "^9.0.0",
+               "@material/radio": "9.0.0",
-               "@material/ripple": "^2.3.0",
-               "@material/select": "^2.3.1",
-               "@material/switch": "^2.3.0",
-               "@material/textfield": "^2.3.1",
+               "@material/ripple": "^9.0.0",
+               "@material/select": "^9.0.0",
+               "@material/switch": "^9.0.0",
+               "@material/textfield": "^9.0.0",
-               "@wordpress/compose": "^3.19.1",
-               "@wordpress/data": "^4.23.0",
+               "@wordpress/compose": "^6.20.0",
+               "@wordpress/data": "^9.0.0",
-               "@wordpress/element": "^2.20.3",
+               "@wordpress/element": "^5.21.0",
-               "react": "^16.14.0",
-               "react-dom": "^16.14.0",
+               "react": "^18.2.0",
+               "react-dom": "^18.2.0",
-               "react-joyride": "^2.3.0",
+               "react-joyride": "^2.6.0",
-               "@testing-library/react": "^10.4.9",
+               "@testing-library/react": "^14.0.0",
-               "@wordpress/babel-preset-default": "^4.17.0",
+               "@wordpress/babel-preset-default": "^7.20.0",
-               "jsdom": "^16.5.0",
+               "jsdom": "^22.1.0",

With all the package version diff, and edits, not so straightforward errors that were shown in tests:

Details

"Warning: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
    1. You might have mismatching versions of React and the renderer (such as React DOM)
    2. You might be breaking the Rules of Hooks
    3. You might have more than one copy of React in the same app
    See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem."],["Warning: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
    1. You might have mismatching versions of React and the renderer (such as React DOM)
    2. You might be breaking the Rules of Hooks
    3. You might have more than one copy of React in the same app
    See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem."
    
"Error: Uncaught [TypeError: Cannot read property 'useContext' of null]    

There might also be a need for Material library to be upgraded to v5, which brings important change in terms that styling is done via styled components, not css. Several currently included react-* packages could be removed in that case, as they are part of the @mui core, and can he imported directly.

That will clear the last remaining packages that have peer dependencies for react 16|17

Details

├─┬ @wordpress/babel-preset-default@7.28.0
│ └── UNMET PEER DEPENDENCY react@18.2.0  deduped
├─┬ @wordpress/data@9.14.0
│ └─┬ @wordpress/element@5.21.0
│   └── react@18.2.0 
├─┬ @wordpress/element@5.21.0
│ └── UNMET PEER DEPENDENCY react@18.2.0  deduped
└── UNMET PEER DEPENDENCY react@18.2.0 

npm ERR! peer dep missing: react@^16.8.0 || ^17.0.0, required by @material-ui/core@4.12.4
npm ERR! peer dep missing: react@^16.4.2, required by @material/react-card@0.15.0
npm ERR! peer dep missing: react@^16.4.2, required by @material/react-chips@0.15.0
npm ERR! peer dep missing: react@^16.4.2, required by @material/react-dialog@0.15.0
npm ERR! peer dep missing: react@^16.4.2, required by @material/react-select@0.15.0
npm ERR! peer dep missing: react@^16.4.2, required by @material/react-tab-bar@0.15.0
npm ERR! peer dep missing: react@^16.4.2, required by @material/react-text-field@0.15.0
npm ERR! peer dep missing: react@^16.8.0, required by @reach/combobox@0.10.5

All except for the @reach/combobox, which has 0.18 as latest version, last update year ago, and not including react 18 as peer dependency, latest is 17.x. Seem this is not yet tested/update up agains react 18

Due to the big jump in versions it will require impactful number of changes and lot more time to research this. Alternative would be using lower version as suggested originally in the issue, but then it would require re-write of a good portion of the codebase to work around useSelect issue, so there would be no nested checks, all would be moved to the top level. To provide work around the issue for which a fix was later introduced.

After going down the rabbit hole, the included errors were last I reached in my research, and they can be caused by a wide number of reasons. I am un-assigning myself, so someone else can take a fresh look.

[kuasha420]

Closing in favor of 8826.