[deps] Update to Electron v12

[matheusd]

This updates electron to the most recent version (v12). Unfortunately, several other dependencies had to be updated alongside it in order to make it work (too much interdependencies between them). OTOH, the win32ipc did not need to be updated.

Summary of changes:

• Update to electron v12.0.1
• Upgrade electron-related deps to their latest version (electron-builder, electron-store, etc)
• Fix dialogs to be called from ipcMain (v12 no longer offers system dialogs from ipcRenderer)
• Update webpack to the latest v4 release
• Update babel to the latest release (this required manually tweaking yarn.lock in some cases)
• Move most deps from app/package.json to package.json (due to some interaction between the newest webpack+babel+electron causing source map warnings in packages defined in app/package.json)
• Drop some requirements that were defined in package.json due to them being transitive requirements instead of direct

Future PRs will look into fixing the trezor-connect dep, upgrading to webpack 5 to keep up with the most recent releases of the major dependencies of the project and using the new security features provided by electron v12.

Tested and working: linux (yarn {dev, build && start, package-dev-linux} ) windows (yarn {dev, package}).

Windows yarn build && yarn start isn't currently working.

[matheusd]

Had to rollback some test dep updates because it broke too much stuff and this is already large. Will work on other updates before trying again to update those.

[amass01]

hrmm i'm totally fine with disabling eslint here if it's complaining but i'm just wondering as we always had react-hooks/exhaustive-deps on but it never complained about this one and tbh it shouldn't.
the fun here is a function param which means it defined inside the scope of useMountEffect hence i would expect eslint to not complain that it's not in the deps array.

[matheusd]

It started complaining after updating one of the dependencies, so a newer linter version is probably being more aggressive in detecting these requirements.

But I'm confused by this:

the fun here is a function param which means it defined inside the scope of useMountEffect

I might be parsing this phrase wrongly, but I'm not sure this statement is correct. const useMountEffect = /*...*/ is not a function definition, but an assignment of a closure.

In any case, it seems to me the linter is in fact correct in complaining here. Consider:

function MyCompo() {
  const [val, setVal] = useState('');
  let f = () => console.log("bleh");
  if val != '' {
    f = () => console.log("blah");
  }

  useMountEffect(f);
  
  // The above is equivalent to
  // useEffect(f, []);

  //...
}

This is wrong in all sorts of ways, but the key bit is that the contents of f var can change arbitrarily between renders (for example, due to the value of the val state), and thus it must be part of the dependency list for the useEffect call.

[amass01]

Hrmm I see what you mean, but I think the deps array is about deps of f not f itself, which in the example above is the actual effect and not one of it's deps.

If f was () => console.log(val) I would expect val to be in the deps array but that isn't the case in the example above.

[matheusd]

I agree with your assessment (which is why the warning is also somewhat confusing to me).

I don't know enough about the underlying React and linter behavior to know for sure the origin of the warning, the above was my guess at what was happening based on useMountEffect using a parametrized function (vs an inline one).

[guilhermemntt]

lodash debounce returns a debounced version of its parameter. To call onChangeSliderCallback after this change will return the debounced function instead of executing it.

[matheusd]

Please see the review fix commit. The simplest way I could see to fix this without resorting to disabling the warning (as in the other issue Amass pointed out) was to switch to useMemo: the inline function now produces the memoized, debounced callback (and correctly recreates it if onChangeSlider itself changes).

It seems to me though, looking at where onChangeSlider is ultimately defined (the HistoryTab component) it's being redefined at every component render there, thus causing unnecessary renders of the menu.

Anyways, hopefully this is correct. I also bumped up the delay a bit. 100ms seems way too small to debounce something that depends on fine use motion of the mouse.

[amass01]

@matheusd That sounds as a good way to approach it, regarding unnecessary re-renders - I would try wrapping onChangeSliderValue in HistoryTab with useCallback which should resolve the redefine/rerender issue.

[amass01]

Tests well

[alexlyp]

tACK