site-kit-wp: Remove legacy code from error stores.
Feature Description
In order to help tidy up the codebase and make the error store code easier to follow, we should remove the legacy usage of the single error
object with no associated baseName
and args
.
Additionally, we should remove the code that adds the selectorData
property in getErrorForSelector()
, as this has been identified as prone to error; it expects the error to be spreadable, but in fact, it’s possible for it to be an Error
instance which cannot be spread into a new object without losing information.
As we now have getSelectorDataForError()
, getErrorForSelector()
should be made analogous to getErrorForAction()
as a simple wrapper to getError()
that provides a bit of additional semantic value.
Any usage of selectorData
retrieved via getErrorForSelector()
should be replaced with getSelectorDataForError()
.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Remove usage of the single
error
object with no associatedbaseName
and args:- The single
error
object should be removed from Redux state. - Any usage of
receiveError()
withoutbaseName
andargs
should be removed (there is only one case of this at the time of writing, but it’s increateFetchStore()
, so it’s being called in a lot of places). - The
baseName
parameter should be mandatory forreceiveError()
,getError()
, andclearError()
, while args can default to an empty array. - All associated legacy code should be removed.
- Ensure the above changes to
createFetchStore()
do not have any impact on current error handling.
- The single
- Update
getErrorForSelector()
to remove the addition ofselectorData
, and return the error as-is. - Replace any usage of
selectorData
retrieved viagetErrorForSelector()
withgetSelectorDataForError()
.
Implementation Brief
- Inside
/assets/js/googlesitekit/data/create-error-store.js
locatereceiveError
,clearError
,getError
functions and make the following changes:- Make the
baseName
parameter to be mandatory forreceiveError
,getError
, andclearError
by usinginvariant
.- Ensure all calls to these functions now receive
baseName
- Ensure all calls to these functions now receive
- Make the
args
parameter to be[]
by default forreceiveError
andclearError
- Change
getErrorForSelector
to return onlyerror
directly. It should not return an object withselectorData
, and not to spread the error variable.
- Make the
- Update existing usage of
const { selectorData } = getErrorForSelector( …args )
toconst selectorData = getSelectorDataForError( …args )
.
Test Coverage
- All existing tests should pass; no new tests are needed.
QA Brief
- There’s no user facing changes .For a general sanity check, make sure all the error states are working and retryable errors are showing the
Retry
Button. - QA:Eng - a secondary round of Code inspection and general test would be useful, specially making sure nothing was missed.
Changelog entry
- Remove legacy error handling code from plugin.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 16
@techanvil Works for me! 👍🏻 I’ll move this back to IB now 🙂
Have created and added AC, please review.
cc @aaemnnosttv