TypeScript: Reduce the amount of optional properties

[victorlin]

from #1864 (comment), #1864 (comment), #1864 (comment), #1864 (comment)

#1854 is set to add types/interfaces with an abundance of optional properties. Example:

auspice/src/components/tree/phyloTree/types.ts

Lines 81 to 126 in 6ca81c6

	export interface PhyloNode extends SVG {
	angle?: number
	branch?: [string, string]
	branchStroke?: string
	conf?: [number, number]
	/** SVG path */
	confLine?: string
	crossDepth?: number
	depth?: number
	displayOrder?: number
	displayOrderRange?: [number, number]
	fill?: string
	inView: boolean
	n: ReduxNode
	pDepth?: number
	// TODO: This should be `string | number`, conditional on layout
	px?: any
	// TODO: This should be `string | number`, conditional on layout
	py?: any
	rot?: number
	smallBigArc?: boolean
	tau?: number
	that: PhyloTreeType
	tipStroke?: string
	update?: boolean
	/** SVG path */
	vaccineCross?: string
	w?: number
	xBase?: number
	xCBarEnd?: number
	xCBarStart?: number
	xCross?: number
	xTip?: number
	yBase?: number
	yCBarEnd?: number
	yCBarStart?: number
	yCross?: number
	yTip?: number
	}

This contributes to the abundance of strictNullChecks violations.

From @jameshadfield in #1864 (review):

For the upcoming work exploring reducing the optional parameters on types let's explore a few ideas at a small scale to better see which approach will be best for us!

Possible solutions

Note: the solution may vary per type/interface

1. Use mapped types
   • I started on this in d08ac2b...12aff24 but dropped it to keep the PR simple.
2. Have a type A with lots of optional properties that we can use as we build up the data structure then turn this into a type B once it arrives in redux and we know all/most properties have been defined. Type predicates may be useful here. (ref)
3. ?

[genehack]

Also worth thinking about ways of handling this on a class level, e.g. this-based type guards.

[victorlin]

Re: #1888 (comment)

As we discussed yesterday, I think the best approach here is to have 2 types per "thing" — one where the properties are largely optional and a second where they're not (insert naming bikeshed convo here...) — and then use type guards to "up-convert" the optional form into the fully reified one.

I would like to try this out on something like ReduxNode first since it has properties such as ReduxNode.children?: ReduxNode. So if there is a ReduxNodeA.children?: ReduxNodeA and ReduxNodeB.children: ReduxNodeB, it's not clear to me how the "up-conversion" will work.

[jameshadfield]

I would like to try this out on something like ReduxNode first since it has properties such as ReduxNode.children?: ReduxNode. So if there is a ReduxNodeA.children?: ReduxNodeA and ReduxNodeB.children: ReduxNodeB, it's not clear to me how the "up-conversion" will work.

I was also interested and gave it a shot this morning in-between meetings:

interface LoadingTypeI {
  foo?: number[]; // example of optional property
  children: LoadingTypeI[];
}

interface LoadedTypeI {
  // Doesn't need to be written out like this, you can use some conditional recursive
  // TS magic to turn LoadingTypeI into this
  foo: number[];
  children: LoadedTypeI[];
}

// (Not shown: we've used a whole bunch of functions to incrementally create the data & now consider it loaded)
const data: LoadingTypeI = {
  foo: [1,2,3],
  children: [
    {foo: [4,5,6], children: []},
    {foo: [7,8,9], children: []},
  ]
}

function isLoadedTypeI(x: LoadingTypeI | LoadedTypeI): x is LoadedTypeI {
  // type predicate <https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates>
  // note that this is not free!
  const properties_to_enforce = ['foo']; // Can use some TS magic to get this from the type itself
  let complete=true;
  const stack=[x]
  while (stack.length) {
    const node = stack.shift()
    if (node===undefined) break; // TS limitation? Is this to do with sparse arrays? I though TS didn't know about them
    for (const child of node.children) {stack.push(child)};
    if (properties_to_enforce.some((p) => node[p]===undefined)) {
      complete=false;
      break;
    }
  }
  return complete;
}

function toRedux(x: LoadingTypeI): LoadedTypeI {
  if (isLoadedTypeI(x)) return x;
  throw new Error(); // make sure this is handled by an error boundary in React
}

function dispatch(arg: any):void {console.log("DISPATCH:", arg)}

dispatch({type: "LOAD", data: toRedux(data)})

toRedux(data).foo.length; // no problem - foo is guaranteed to exist

[jameshadfield]

I played around with this after resurrecting an old prototype of adding types to reduxState.metadata, essentially defining a type for the "complete" state intended to be used by rendering components etc:

interface MetadataState {
  loaded: true; // discriminant property
  title?: string;
  mainTreeNumTips: number;
  identicalGenomeMapAcrossBothTrees: boolean;

and a "builder" type used when parsing the JSON and also as the default redux state:

type IncompleteMetadataState = Partial<Omit<MetadataState, "loaded">> & {loaded: false};

At the end of dataset loading we run something like

function convertIncompleteMetadataStateToMetadataState(meta: IncompleteMetadataState): MetadataState 

which will throw if the metadata is incomplete such that Auspice can't visualise the data. Within components we can then use the discriminant property to narrow the type, essentially asserting everything is as is should be:

const mapState = (state: RootState) => {
  if (!state.metadata.loaded) throw new Error("Something's gone seriously wrong")
  return {metadata: state.metadata}
}

It all felt a little bit hard. I'm sure it can be simplified. But the overall result is the ability to simplify rendering components as we don't need to constantly check a property is defined or not.