mito icon indicating copy to clipboard operation
mito copied to clipboard

useSyncedParams improvements

Open aarondr77 opened this issue 2 years ago • 3 comments

usedSyncParams is a powerful centralized hook that we can utilize to make improvements across the codebase. Here are a few things that we should consider adding:

Results from the backend

As part of our work to improve communication about the results of actions taken by the user, we want to be able to retrieve key information about the step from the backend and display it to the user. For examples

  • Tell the user how many rows were removed when they apply a filter
  • Tell the user how many of their keys were matched, and how many were unmatched in a merge
  • Tell the user which columns were not matched when they performed an inner concat

We should add a results field to the usedSyncParams that returns an object typed specifically for each step. For example, the filter results object might be {numRowsRemoved: number}, whereas the concat results object might be `{sheetIndex: {numColumnsUnmatched: number, unmatchedColumnIDs: [columnID]}}

In the frontend of each step, we can use these params to give the user some helpful info that will help them understand the results of the actions they've taken.

Handing undefined values to prevent sheet crashing bugs

The largest source, maybe the only source, of sheet crashing bugs is indexing into undefined values. For example, a common place that this occurs is when the using Undo. Take the example of importing two sheets, and then opening the deduplication taskpane. If you then undo twice in order to get rid of the second sheet, the sheet crashes (just for the next 5 minutes) because the params.sheet_index of the concat params is no longer in the sheetDataArray.

Now that we have a centralized place to handle all of the step params, we can do safety checking when an undo event occurs to reset the params if they don't exist anymore!

aarondr77 avatar Mar 28 '22 15:03 aarondr77

@aarondr77 because we use the sheetDataArray in places other than the taskpanes, and we have sheet crashing bugs all over as a result, IMO, the best solution is to define sheetDataArray as also optionally having undefined values (as well as other state values)

This would take a lot of work to fix up properly, I think, but it also might not be too much. Just making sure all the type checking works is tough.

naterush avatar Mar 28 '22 15:03 naterush

The results from the backend sound awesome!

naterush avatar Mar 28 '22 15:03 naterush

Other things we could include:

  1. Processing time.
  2. The ability to set a timeout on the operation so that it will quit after x seconds (or prompt the user to quit or something). This requires backend changes I don't know how to make currently - namely interrupting an operation.

naterush avatar Mar 28 '22 15:03 naterush