feat: auto-update Mapeo when internet is available

[okdistribute]

Contributor checklist:

• My contribution is not related to translations. Please submit translation changes via our Mapeo Crowdin project
• My commits are in nice logical chunks with good commit messages
• I have walked through the QA Manual Testing
  Script and updated it if necessary.
• If my changes depend upon an update to a dependency, I have updated the package-lock.json file using npm install --package-lock
• My changes are ready to be shipped to users on Windows, Mac, and Linux

Description

Here's a video demo. I used the existing tabs on the left because it was easier and I didn't have to create any new styles, and would probably be much nicer that whatever I would have made customized.

TODO

• Decide what we want to change about this design.
• Blank moment between clicking 'download' and seeing download progress
• CSS wonkiness on sidebar during download, need to fix the size
  - [ ] Show release notes~
• Use AWS bucket for release uploads and downloads https://downloads.mapeo.app
• Test beta release/downgrade workflow

[gmaclennan]

Great work on this @okdistribute this is really slick and will be great for Mapeo users.

I don't think it makes sense to add this as a separate tab, I think all the information and interaction needed can be shown within the sidebar (e.g. it does not need to show anything in the main app space), although I appreciate that the current implementation makes styling easier. I'm happy to integrate as-is and update styling later. When I get a moment I can sketch something out.

I do think we need to change the CSS so that the width of the update item does not alter the width of the sidebar. It should respond to the sidebar width. Changing the sidebar width will slow down the rest of the app on slower machines (because of redrawing the map screen, and I'm not sure we actually listen for changes in width, only changes in window size, so it might lead to rendering artifacts) and be difficult for users trying to use the app during update because the screen will jump around.

[okdistribute]

OK @gmaclennan, fixed the CSS. Can look at putting information inside the sidebar, wanted to run this by everyone to see what additional information is needed there.

[gmaclennan]

Keeping update state in the <Home> component is going to cause a re-render of the whole app for each update event (e.g. progress events). This may affect performance of the app when using it during an update. It could be resolved by memoizing the other components (territory and observation views) if they are not already, or it could be solved by placing the updater state in react context. It would be worth checking the performance / re-render cost with latest react devtools to see whether there is a performance cost.

[okdistribute]

Good point, although I don't know enough about these details so I'll need to read up on it a bit

[gmaclennan]

This could lead to tab labels being truncated in different languages

[okdistribute]

Do you have a suggested way to fix this that doesn't cause the sidebar to jump around?

[gmaclennan]

Great work on this @okdistribute. I think there are ways that the API wrapper for electron-updater and communication between the processes could be changed reduce bugs and simplify debugging. Currently interaction over IPC via requests with no response and event listeners for the error or progress events make it hard to render the UI to respond to user events e.g. show progress as soon as the user clicks the "download" button, because the UI state and the server state are not in sync.

One way to solve this is to set up async communication between the renderer process and the updater. I'm not sure if this will work via remote.require() or we need to set up our own ipc. E.g. an alternative version of useUpdater.js (This isn't working code, just an example to try to explain what I am thinking):

import { STATES } from "./";
import electronIpc from "../../electron-ipc";
import { remote } from "electron";
import { useEffect, useState } from "react";

const autoUpdate = remote.require("../../main/someElectronUpdaterWrapper");

export default function useUpdater() {
  const [update, setUpdate] = useState({
    progress: null,
    updateInfo: null,
    state: STATES.IDLE,
  });

  function checkForUpdates() {
    if (update.state === STATES.PROGRESS) return;
    autoUpdate
      .checkForUpdates()
      .then((info) =>
        setUpdate({
          updateInfo: info,
          progresss: null,
          state: STATES.AVAILABLE,
        })
      )
      .catch((e) =>
        setUpdate({ updateInfo: null, progress: null, state: STATES.ERROR })
      );
    setUpdate({
      updateInfo: null,
      progress: null,
      state: STATES.CHECKING_FOR_UPDATE, // New state
    });
  }

  function downloadUpdate() {
    if (update.state !== STATES.AVAILABLE) return;

    function onProgress(progress) {
      setUpdate((state) => ({ ...state, progress }));
    }

    autoUpdate.on("download-progress", onProgress);

    autoUpdate
      .downloadUpdate()
      .then(() =>
        setUpdate(state => ({
          updateInfo: state.info,
          progresss: null,
          state: STATES.READY_FOR_RESTART,
        }))
      )
      .catch((e) =>
        setUpdate({ updateInfo: null, progress: null, state: STATES.ERROR })
      )
      .finally(() => autoUpdate.off("download-progress"));

    setUpdate({
      updateInfo: null,
      progress: 0, // Whatever progress state is at zero
      state: STATES.PROGRESS
    });
  }

  return [update, { checkForUpdates, downloadUpdate }];
}

There are some caveats to using remote.require() and I'm not sure if it supports promises in electron@6, so we might need to do our own thing with ipcRenderer, which also does not seem to support invoke() in electron@6 :(

Without setting state in response to user events, the UI will be unresponsive and users may click buttons multiple times thinking it is not working. I'm not sure if calling autoupdate.downloadUpdate() multiple times causes multiple downloads?

Finally, I think @xstate/fsm can be super helpful for implementing this kind of thing, although there is a learning curve. It helps catch all the edge cases and simplifies implementation, but using the switch statements as you have in the components works just as well and is fine.

[okdistribute]

Great work on this @okdistribute. I think there are ways that the API wrapper for electron-updater and communication between the processes could be changed reduce bugs and simplify debugging. Currently interaction over IPC via requests with no response and event listeners for the error or progress events make it hard to render the UI to respond to user events e.g. show progress as soon as the user clicks the "download" button, because the UI state and the server state are not in sync.

Showing progress as soon as the user clicks the download button is already captured in UpdaterView when the user clicks the button. Perhaps best to move all state management code to the useUpdater file.

One way to solve this is to set up async communication between the renderer process and the updater. I'm not sure if this will work via remote.require() or we need to set up our own ipc. E.g. an alternative version of useUpdater.js (This isn't working code, just an example to try to explain what I am thinking):

Creating an asyncronous electron-ipc listener is already supported out of the box with electron v6, no need for remote.require() or our own ipc, using event.reply.

https://github.com/electron/electron/blob/v6.0.10/docs/api/ipc-main.

import { STATES } from "./";
import electronIpc from "../../electron-ipc";
import { remote } from "electron";
import { useEffect, useState } from "react";

const autoUpdate = remote.require("../../main/someElectronUpdaterWrapper");

export default function useUpdater() {
  const [update, setUpdate] = useState({
    progress: null,
    updateInfo: null,
    state: STATES.IDLE,
  });

  function checkForUpdates() {
    if (update.state === STATES.PROGRESS) return;
    autoUpdate
      .checkForUpdates()
      .then((info) =>
        setUpdate({
          updateInfo: info,
          progresss: null,
          state: STATES.AVAILABLE,
        })
      )
      .catch((e) =>
        setUpdate({ updateInfo: null, progress: null, state: STATES.ERROR })
      );
    setUpdate({
      updateInfo: null,
      progress: null,
      state: STATES.CHECKING_FOR_UPDATE, // New state
    });
  }

  function downloadUpdate() {
    if (update.state !== STATES.AVAILABLE) return;

    function onProgress(progress) {
      setUpdate((state) => ({ ...state, progress }));
    }

    autoUpdate.on("download-progress", onProgress);

    autoUpdate
      .downloadUpdate()
      .then(() =>
        setUpdate(state => ({
          updateInfo: state.info,
          progresss: null,
          state: STATES.READY_FOR_RESTART,
        }))
      )
      .catch((e) =>
        setUpdate({ updateInfo: null, progress: null, state: STATES.ERROR })
      )
      .finally(() => autoUpdate.off("download-progress"));

    setUpdate({
      updateInfo: null,
      progress: 0, // Whatever progress state is at zero
      state: STATES.PROGRESS
    });
  }

  return [update, { checkForUpdates, downloadUpdate }];
}

There are some caveats to using remote.require() and I'm not sure if it supports promises in electron@6, so we might need to do our own thing with ipcRenderer, which also does not seem to support invoke() in electron@6 :(

Without setting state in response to user events, the UI will be unresponsive and users may click buttons multiple times thinking it is not working. I'm not sure if calling autoupdate.downloadUpdate() multiple times causes multiple downloads?

This state is prevented in the frontend in UpdaterView as mentioned before, but I'll make sure that if it happens to be called twice somehow that it won't download twice (although I hope this is caught by Electron Updater!?)

Finally, I think @xstate/fsm can be super helpful for implementing this kind of thing, although there is a learning curve. It helps catch all the edge cases and simplifies implementation, but using the switch statements as you have in the components works just as well and is fine.

Seems like a larger refactoring project, perhaps we can discuss if we want to do this later?

[okdistribute]

To add, I don't think we want to turn on and off updates for download-progress events because otherwise the UI state could diverge from the back-end state. Any back-end events should always be rendered in the frontend