Description
openedon Feb 22, 2021
Epic: https://github.com/elastic/security-team/issues/1972
Summary
TL;DR:
- Keep frontend app logic outside of React components.
- Keep it consolidated in React contexts with its own state and exposed high-level actions.
- Use high-level mechanisms for building app logic (e.g. React Query, XState) instead of over-relying on low-level hooks (e.g.
useState
,useCallback
,useEffect
). - Build high-level abstractions clearly describing what the app can do and what it can't do (i.e. what components are not supposed to be doing).
- Be able to test app logic without unit testing large components like tables and pages.
- Be able to test app logic without writing an exhaustive suite of e2e Cypress tests.
- Be able to cover app logic scenarios with unit tests.
Our implementation of the Rule Management and Monitoring tables, as well as whole Rule Management and Rule Details pages, is pretty hairy, with pieces of the application logic being scattered across components, over-reliance on primitive React hooks to manage state and logic (useState
, useCallback
, etc), not using context and/or store, lack of a single source of truth for state (could be a reducer or a similar thing), duplication of state in hooks and components, overuse of flags here and there and thus complex boolean checks like !initLoading && (loading || isLoadingRules || isLoadingAnActionOnRule) && !isRefreshing
, underuse of clearly and explicitly defined states, components overloaded with dependencies and inline logic which leads to testing client-side app logic being very difficult and brittle, so we skip all kinds of lower-level testing and add only Cypress tests.
Recently, we have been switching to React Query to manage the Rules table's state and the cache of loaded rules and made great progress so far. However, there's still a lot to do before we can consider all the business logic of rule management extracted from React components, which should in turn give us the ability to cover it with unit tests.
To do
- Extract rule management logic that doesn't depend on the page (Rule Management, Rule Details, Rule Editing, etc) to separate React contexts. Examples of such logic: enabling/disabling an individual rule, creating a rule, editing a rule, deleting a rule, etc
- Move the whole Rules table logic to the existing
RulesTableContext
. Expose higher-level actions fromRulesTableActions
likedeleteRule
,deleteSelectedRules
,enableRule
,enableSelectedRules
, etc, and hide lower-level actions likesetLoadingRules
. The logic should determine some of its internal states like loading indicators on its own, without delegating it to components. - Extract logic of loading rule details (for the Rule Details page) to its own React context.