silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 91 forks source link

Replace redux-form with react-hook-form #1755

Closed emteknetnz closed 3 weeks ago

emteknetnz commented 1 month ago

Implementation of the spike which recommends that we continue replacing redux-form with react-hook-form

In the linked spire there are a few PRs that have successfully replaced redux-form with react-hook-form, there's a number of loose ends to tidy up e.g. no longer using redux-form helpers to update the redux store

Acceptance criteria

Notes

PRs

POC PRs

emteknetnz commented 3 weeks ago

In the PRs above I've swapped out redux-form for react-hook-form and run it through a CI run of kitchen-sink including all of the behat tests, and there were a fairly large number and wide range of failures. I had a decent look at one of the failures and it was because the AnchorSelectorField relies on the redux state set by redux-form of an entirely different react component in order to function.

Currently, the form state is updated by reducer provided by redux-form, which updates window.ss.store.getState().form.formState. This reducer is connected in registerReducers.js.

In order to fix all of the behat failures, we'd probably need to replicate what the redux-form reducer does, though seems like a pretty poor use of time. Not only that, this would be adding more complexity to an already convoluted tech stack and actually worsen the current situation.

A better approach would be to aim for removing Redux from our tech stack entirely.

This would change the approach taken here. Instead of starting by swapping redux-form with react-hook-form, we should keep redux-form temporarily and gradually refactor Redux out of individual React components, replacing it with regular component state. Once all components are refactored, the redux-form updates to Redux state become irrelevant. At that point, we can replace redux-form with react-hook-form.

Refactoring all of the react components that use redux is a massive increase in scope. While I think it's worthwhile long term to improve the code base, realistically it's hard to justify the opportunity cost.

A quick search in silverstripe for mapStateToProps in ./vendor/silverstripe/**/src/* yields 32 matches, so it's a large epic sized job.

Feature files that failed on kitchen-sink ci

admin

asset-admin

campaign-admin

cms

elemental:

elemental-bannerblock

linkfield