Feature/type metadata branch #91
Conversation
| if (feature && feature.discrete) { | ||
| const categoryValues = map(feature.options, (_, key) => Number(key)); | ||
| const totals = reduce( | ||
| measuredData[categoryToColorBy], | ||
| measuredData.values[categoryToColorBy], |
There was a problem hiding this comment.
this was a mistake I found by correctly typing everything
…ature-explorer into feature/type-metadata-branch
| }, | ||
| measuredFeaturesDefs: [], | ||
| viewerChannelSettings: undefined, | ||
| viewerChannelSettings: {} as ViewerChannelSettings, |
There was a problem hiding this comment.
should initialState be typed as MetadataStateBranch?
export const initialState: MetadataStateBranch = {
There was a problem hiding this comment.
it can be, it gets typed in State at the creation of the store, so if anything is mistyped here it would cause an error, but I can add it here for clarity
| labels: PerCellLabels; | ||
| }; | ||
| measuredFeaturesDefs: MeasuredFeatureDef[]; | ||
| viewerChannelSettings: ViewerChannelSettings; |
| export const getPlotByOnX = (state: State): string => state.selection.plotByOnX; | ||
| export const getPlotByOnY = (state: State): string => state.selection.plotByOnY; | ||
| export const getGroupByCategory = (state: State): string => state.selection.groupBy; | ||
| export const getClickedCellsFileInfo = (state: State): FileInfo[] => state.selection.selectedPoints; |
There was a problem hiding this comment.
Wow, are the return types not automatically inferred/enforced if we don't specify them here? Now that SelectionStateBranch is typed?
There was a problem hiding this comment.
I think they are, I didn't check, but I thought it would be a good rule to just have every selector have a return type because it makes it clearer when you look at the selector what you're getting
src/state/metadata/actions.ts
Outdated
| @@ -59,15 +60,15 @@ export function receiveFileInfoData(payload: FileInfo[]): ReceiveCellFileInfoAct | |||
| }; | |||
| } | |||
|
|
|||
| export function receiveMetadata(payload: MetadataStateBranch): ReceiveAction { | |||
| export function receiveDataForPlot(payload: DataForPlot): ReceiveAction { | |||
| return { | |||
| payload, | |||
| type: RECEIVE_METADATA, | |||
There was a problem hiding this comment.
Should RECEIVE_METADATA also be renamed for clarity?
| @@ -157,12 +160,14 @@ const INITIAL_COLOR_AND_GROUP_BY = "cell-line"; | |||
|
|
|||
There was a problem hiding this comment.
Can featureData above (ln 140) be typed as DataForPlot now?
Problem
In the last PR, both @schoinh and @toloudis suggested typing things that ended up introducing a lot more changes.
Solution
Pulled this into a separate PR with these changes:
selectionStateBranchandmetaStateBranchType of change
Please delete options that are not relevant.
Keyfiles (delete if not relevant):
Start with all the
selectors.tsfiles and the state types: src/state/selection/types.ts, src/state/metadata/types.ts