Skip to content

refactor: use union types to better represent discussions

Tomas Vik requested to merge 317-prepare-for-discussion-resolving into main

This MR introduces a small refactoring that uses the TS type system to represent the Discussion type pore completely.

We have three distinct discussions:

  • overview discussions, all notes in these discussions have position: null
  • image discussions, all notes in these discussions have position: { type: 'image' }
  • text diff discussions, all notes in these discussions have position: { type: 'text' }
    • text diff discussions are further divided into discussions on the old diff and the new diff.

We represented all these possibilities in one type and so we had to do null checks throughout the code because TS couldn't help us distinguish between the three.

This MR says that discussion is either overview discussion, discussion on an image, or discussion on text diff.

This MR introduces more TS-specific code that might be harder to understand for JS developers. I have a marginal preference for more specific types to help TS help us, but I could be nudged towards dropping this change.

Related to #317 (closed)

Edited by Tomas Vik

Merge request reports