elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.74k stars 8.14k forks source link

[Response Ops] Rule.actions state mutation in Alerts Form #155993

Open jcger opened 1 year ago

jcger commented 1 year ago

Issue

Our ActionForm component is mutating the rule.actions state by pushing elements into it, instead of using React's callback to update the state. This is something we should avoid, as explained in the React Documentation:

In JavaScript, arrays are just another kind of object. [Like with objects](https://react.dev/learn/updating-objects-in-state), you should treat arrays in React state as read-only. This means that you shouldn’t reassign items inside an array like arr[0] = 'bird', and you also shouldn’t use methods that mutate the array, such as push() and pop().

Current scenario

Rule definition as a state, one of its props is actions. (source code link)

  ...
  const [{ rule }, dispatch] = useReducer(ruleReducer as InitialRuleReducer, {
    rule: initialRule,
  });
  ... 

rule.action being passed by to ActionForm component (source code link)

  ...
  <ActionForm
     actions={rule.actions}
     setHasActionsDisabled={setHasActionsDisabled}
   ...

rule.action being mutated inside the ActionForm component several times, for example (source code link)

  ...
  actions.push({
    id: '',
    actionTypeId: actionTypeModel.id,
    group: defaultActionGroupId,
    params: {},
     frequency: defaultRuleFrequency,
  });

Expected outcome

We would refactor the code so it calls the dispatcher function instead of mutating the actions array

elasticmachine commented 1 year ago

Pinging @elastic/response-ops (Team:ResponseOps)