New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade typescript to latest version, and fix new errors #3466
Upgrade typescript to latest version, and fix new errors #3466
Conversation
Signed-off-by: Adam Abeshouse <abeshoua@mskcc.org>
@@ -273,8 +273,10 @@ export enum SampleListCategoryType { | |||
} | |||
|
|||
export enum GeneticEntityType { | |||
'GENE' = 'gene', | |||
'GENESET' = 'geneset', | |||
'GENE' = 'GENE', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dippindots you worked on geneset stuff right? could you take a look at these changes I made relating to GeneticEntityType and make sure I haven't done anything weird? this is all just to fix typescript errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamabeshouse This looks good to me! It's in sync with the backend.
@@ -611,7 +611,7 @@ export function getAlterationEnrichmentColumns( | |||
enrichedGroupColum.tooltip = ( | |||
<table> | |||
<tr> | |||
<td>Log ratio > 0</td> | |||
<td>Log ratio {'>'} 0</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
officially, JSX does not allow >, <, {, } as plaintext characters, and the new version of typescript catches that
@@ -707,8 +710,10 @@ export function annotateGeneticTrackData( | |||
const newObj = _.clone(object); | |||
newObj.data = newObj.data.filter(d => { | |||
// clear previous annotations | |||
delete d.oncoKbOncogenic; | |||
delete d.putativeDriver; | |||
delete (d as Partial<OncoprinterGeneticTrackDatum_Data>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new typescript has a check which does not allow you to use delete
on a member which is not potentially undefined in the type
abstract _session: MobxPromise<Session>; | ||
abstract _originalGroups: MobxPromise<ComparisonGroup[]>; | ||
abstract overlapStrategy: OverlapStrategy; | ||
abstract usePatientLevelEnrichments: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new typescript catches if the superclass declares an instance property and the subclass overrides it using a get
accessor, so after some difficulty I realized that the solution is to declare these as abstract properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
abstract forces the subclass to define it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so, yes. and it allows the property to be implemented using an instance property or a getter
replaceGene={this.props.replaceGene} | ||
{...this.props} | ||
/> | ||
<GeneSymbolValidatorMessageChild {...this.props} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happened to replaceGene?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's already in this.props
No description provided.