Skip to content
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

Merged
merged 1 commit into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@
"tslib": "^1.11.1",
"typed-css-modules": "^0.6.4",
"typed-css-modules-webpack-plugin": "^0.1.3",
"typescript": "3.8.3",
"typescript": "4.0.3",
"underscore": "^1.8.3",
"url": "^0.11.0",
"url-loader": "^1.1.2",
Expand Down
2 changes: 1 addition & 1 deletion src/pages/patientView/vafPlot/ThumbnailExpandVAFPlot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class ThumbnailExpandVAFPlot extends React.Component<
overlay={<VAFPlot {...expandedProps} />}
arrowContent={<div className="rc-tooltip-arrow-inner" />}
destroyTooltipOnHide={false}
mouseLeaveDelay={isWebdriver ? 2 : 0.05}
mouseLeaveDelay={isWebdriver() ? 2 : 0.05}
>
<div className={this.props.cssClass || ''}>
<VAFPlot {...thumbnailProps} />
Expand Down
6 changes: 4 additions & 2 deletions src/pages/resultsView/ResultsViewPageStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,10 @@ export enum SampleListCategoryType {
}

export enum GeneticEntityType {
'GENE' = 'gene',
'GENESET' = 'geneset',
'GENE' = 'GENE',
Copy link
Contributor Author

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

Copy link
Member

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.

'GENESET' = 'GENESET',
'GENERIC_ASSAY' = 'GENERIC_ASSAY',
'PHOSPHOPROTEIN' = 'PHOSPHOPROTEIN',
}

export const SampleListCategoryTypeToFullId = {
Expand Down
1 change: 0 additions & 1 deletion src/pages/resultsView/coExpression/CoExpressionTabUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export type CoExpressionWithEntityInfo = CoExpression & {
};

export type CoExpressionWithQ = CoExpressionWithEntityInfo & {
geneticEntityType: GeneticEntityType;
qValue: number;
};

Expand Down
2 changes: 1 addition & 1 deletion src/pages/resultsView/coExpression/CoExpressionViz.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ export default class CoExpressionViz extends React.Component<
geneticEntityName: this.highlightedCoExpression
.geneticEntityName,
geneticEntityType: this.highlightedCoExpression
.geneticEntityType,
.geneticEntityType as GeneticEntityType,
geneticEntityId: this.highlightedCoExpression
.geneticEntityId,
cytoband: this.highlightedCoExpression.cytoband,
Expand Down
6 changes: 3 additions & 3 deletions src/pages/resultsView/enrichments/EnrichmentsUtil.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ export function getAlterationEnrichmentColumns(
enrichedGroupColum.tooltip = (
<table>
<tr>
<td>Log ratio > 0</td>
<td>Log ratio {'>'} 0</td>
Copy link
Contributor Author

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

<td>: Enriched in {group1.name}</td>
</tr>
<tr>
Expand Down Expand Up @@ -728,7 +728,7 @@ export function getEnrichmentColumns(
enrichedGroupColum.tooltip = (
<table>
<tr>
<td>Log ratio > 0</td>
<td>Log ratio {'>'} 0</td>
<td>: Enriched in {group1.name}</td>
</tr>
<tr>
Expand Down Expand Up @@ -877,7 +877,7 @@ export function getGenericAssayEnrichmentColumns(
enrichedGroupColum.tooltip = (
<table>
<tr>
<td>Log ratio > 0</td>
<td>Log ratio {'>'} 0</td>
<td>: Enriched in {group1.name}</td>
</tr>
<tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export default class MutualExclusivityTable extends React.Component<
tooltip: (
<table>
<tr>
<td>Log2 ratio > 0</td>
<td>Log2 ratio {'>'} 0</td>
<td>: Tendency towards co-occurrence</td>
</tr>
<tr>
Expand Down
9 changes: 5 additions & 4 deletions src/pages/resultsView/plots/PlotsTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4935,8 +4935,9 @@ export default class PlotsTab extends React.Component<IPlotsTabProps, {}> {
<div style={{ marginTop: 5 }}>
<div>
** Labeling of threshold values (e.g.
>8.00) excludes threshold values from
correlation coefficient calculation.
{'>'}8.00) excludes threshold values
from correlation coefficient
calculation.
</div>
</div>
)}
Expand All @@ -4945,8 +4946,8 @@ export default class PlotsTab extends React.Component<IPlotsTabProps, {}> {
<div style={{ marginTop: 5 }}>
<div>
** Labeling of threshold values (e.g.
>8.00) excludes threshold values from
box plot calculation.
{'>'}8.00) excludes threshold values
from box plot calculation.
</div>
</div>
)}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/resultsView/settings/DriverAnnotationControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export default class DriverAnnotationControls extends React.Component<
onClick={this.onInputClick}
data-test="annotateCBioPortalCount"
/>
cBioPortal >=
cBioPortal {'>='}
</label>
<EditableSpan
value={
Expand Down Expand Up @@ -306,7 +306,7 @@ export default class DriverAnnotationControls extends React.Component<
onClick={this.onInputClick}
data-test="annotateCOSMICCount"
/>
COSMIC >=
COSMIC {'>='}
</label>
<EditableSpan
value={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import {
GeneticTrackDatum_Data,
} from '../../../../shared/components/oncoprint/Oncoprint';
import { percentAltered } from '../../../../shared/components/oncoprint/OncoprintUtils';
import { AlterationTypeConstants } from '../../../resultsView/ResultsViewPageStore';
import {
AlterationTypeConstants,
AnnotatedExtendedAlteration,
} from '../../../resultsView/ResultsViewPageStore';
import { cna_profile_data_to_string } from '../../../../shared/lib/oql/AccessorsForOqlFilter';
import {
fillGeneticTrackDatum,
Expand Down Expand Up @@ -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>)
Copy link
Contributor Author

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

.oncoKbOncogenic;
delete (d as Partial<OncoprinterGeneticTrackDatum_Data>)
.putativeDriver;
// annotate and filter out if necessary
switch (d.molecularProfileAlterationType) {
case AlterationTypeConstants.COPY_NUMBER_ALTERATION:
Expand Down
4 changes: 3 additions & 1 deletion src/pages/staticPages/tools/oncoprinter/OncoprinterHelp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ export const ClinicalFormatHelp = (
<code>Age({ClinicalTrackDataType.NUMBER})</code>&#9;
<code>Cancer_Type({ClinicalTrackDataType.STRING})</code>&#9;
<code>Mutation_Count({ClinicalTrackDataType.LOG_NUMBER})</code>&#9;
<code>Mutation_Spectrum(C>A/C>G/C>T/T>A/T>C/T>G)</code>
<code>
Mutation_Spectrum(C{'>'}A/C{'>'}G/C{'>'}T/T{'>'}A/T{'>'}C/T{'>'}G)
</code>
<br />
Each following row gives the sample id, then the value for each clinical
attribute, or the special value {ONCOPRINTER_VAL_NA} which indicates
Expand Down
10 changes: 6 additions & 4 deletions src/pages/studyView/StudyViewPageStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ export class StudyViewPageStore {
);
//studyViewFilter can only have studyIds or sampleIdentifiers
if (!_.isEmpty(studyViewFilter.sampleIdentifiers)) {
delete studyViewFilter.studyIds;
delete (studyViewFilter as Partial<StudyViewFilter>).studyIds;
}

studyViewFilter.patientTreatmentFilters = { filters: [] };
Expand Down Expand Up @@ -2291,7 +2291,7 @@ export class StudyViewPageStore {
if (bins.length > 0) {
newFilter.customBins = bins;
} else {
delete newFilter.customBins;
delete (newFilter as Partial<ClinicalDataBinFilter>).customBins;
}
this._clinicalDataBinFilterSet.set(uniqueKey, newFilter);
}
Expand Down Expand Up @@ -4945,9 +4945,11 @@ export class StudyViewPageStore {
studyViewFilter.sampleIdentifiers = sampleIdentifiersFromGenomicProfileFilter;
// only one of [studyIds, sampleIdentifiers] should present in studyViewFilter.
// sending both would throw error.
delete studyViewFilter.studyIds;
delete (studyViewFilter as Partial<StudyViewFilter>)
.studyIds;
}
delete studyViewFilter.genomicProfiles;
delete (studyViewFilter as Partial<StudyViewFilter>)
.genomicProfiles;
}

return internalClient.fetchFilteredSamplesUsingPOST({
Expand Down
4 changes: 3 additions & 1 deletion src/shared/api/urls.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { assert } from 'chai';
import { shallow, mount } from 'enzyme';
import sinon from 'sinon';
import AppConfig from 'appConfig';
import { IServerConfig } from 'config/IAppConfig';

describe('url library', () => {
before(() => {
Expand All @@ -13,7 +14,8 @@ describe('url library', () => {
});

after(() => {
delete AppConfig.serverConfig.genomenexus_url;
delete (AppConfig.serverConfig as Partial<IServerConfig>)
.genomenexus_url;
delete AppConfig.apiRoot;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,7 @@ export default class GeneSymbolValidatorMessage extends React.Component<

return (
<div id="geneBoxValidationStatus">
<GeneSymbolValidatorMessageChild
replaceGene={this.props.replaceGene}
{...this.props}
/>
<GeneSymbolValidatorMessageChild {...this.props} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened to replaceGene?

Copy link
Contributor Author

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

</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,25 @@ import { getDefaultCancerCellFractionColumnDefinition } from './CancerCellFracti
import CancerCellFractionElement from 'shared/components/mutationTable/column/cancerCellFraction/CancerCellFractionElement';
import SampleManager from 'pages/patientView/SampleManager';
import { initMutation } from 'test/MutationMockUtils';
import { Mutation } from 'cbioportal-ts-api-client';

describe('CancerCellFractionColumnFormatter', () => {
function createMutationWithoutASCN() {
let mutationWithoutASCN = initMutation({
sampleId: 'S003',
});
delete mutationWithoutASCN.alleleSpecificCopyNumber;
delete (mutationWithoutASCN as Partial<Mutation>)
.alleleSpecificCopyNumber;
return mutationWithoutASCN;
}

function createMutationWithoutCCF() {
let mutationWithoutCCF = initMutation({
sampleId: 'S002',
});
delete mutationWithoutCCF.alleleSpecificCopyNumber.ccfExpectedCopies;
delete (mutationWithoutCCF.alleleSpecificCopyNumber as Partial<
Mutation['alleleSpecificCopyNumber']
>).ccfExpectedCopies;
return mutationWithoutCCF;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ describe('ClonalColumnFormatter', () => {
minorCopyNumber: 1,
},
});
delete emptyMutation.alleleSpecificCopyNumber.clonal;
delete emptyMutation.alleleSpecificCopyNumber.ccfExpectedCopies;
delete emptyMutation.alleleSpecificCopyNumber;
delete (emptyMutation.alleleSpecificCopyNumber as Partial<
Mutation['alleleSpecificCopyNumber']
>).clonal;
delete (emptyMutation.alleleSpecificCopyNumber as Partial<
Mutation['alleleSpecificCopyNumber']
>).ccfExpectedCopies;
delete (emptyMutation as Partial<Mutation>).alleleSpecificCopyNumber;
return emptyMutation;
}

Expand Down
7 changes: 5 additions & 2 deletions src/shared/lib/MutationUtils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -761,11 +761,14 @@ describe('MutationUtils', () => {
const mutationWithoutASCNProperty = initMutation({
sampleId: 'P1_sample1',
});
delete mutationWithoutASCNProperty.alleleSpecificCopyNumber.ascnMethod;
delete (mutationWithoutASCNProperty.alleleSpecificCopyNumber as Partial<
Mutation['alleleSpecificCopyNumber']
>).ascnMethod;
const mutationWithoutASCN = initMutation({
sampleId: 'P1_sample1',
});
delete mutationWithoutASCN.alleleSpecificCopyNumber;
delete (mutationWithoutASCN as Partial<Mutation>)
.alleleSpecificCopyNumber;

it('checks if mutation has allele specific copy number and specified sub-property', () => {
const hasASCNMethod = hasASCNProperty(
Expand Down
6 changes: 2 additions & 4 deletions src/shared/lib/URLWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ export type Property<T, NestedObjectType = any> = {
aliases?: string[];
};

export function needToLoadSession(
obj: Partial<ResultsViewURLWrapper>
): boolean {
export function needToLoadSession(obj: Partial<URLWrapper<any>>): boolean {
return (
obj.sessionId !== undefined &&
obj.sessionId !== 'pending' &&
Expand Down Expand Up @@ -409,7 +407,7 @@ export default class URLWrapper<
return getRemoteSession(this.sessionId);
}

@computed get needToLoadSession() {
@computed get needToLoadSession(): boolean {
// if we have a session id
// it's NOT equal to pending
// we either have NO session data or the existing session data is
Expand Down
17 changes: 8 additions & 9 deletions src/shared/lib/comparison/ComparisonStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export enum OverlapStrategy {
EXCLUDE = 'Exclude',
}

export default class ComparisonStore {
export default abstract class ComparisonStore {
private tabHasBeenShown = observable.map<boolean>();
private tabHasBeenShownReactionDisposer: IReactionDisposer;
@observable public newSessionPending = false;
Expand Down Expand Up @@ -170,6 +170,13 @@ export default class ComparisonStore {
protected async saveAndGoToSession(newSession: Session) {
throw new Error(`saveAndGoToSession must be implemented in subclass`);
}
abstract _session: MobxPromise<Session>;
abstract _originalGroups: MobxPromise<ComparisonGroup[]>;
abstract overlapStrategy: OverlapStrategy;
abstract usePatientLevelEnrichments: boolean;
Copy link
Contributor Author

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

Copy link
Collaborator

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?

Copy link
Contributor Author

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

abstract samples: MobxPromise<Sample[]>;
abstract studies: MobxPromise<CancerStudy[]>;
// < / >

public get isLoggedIn() {
return this.appStore.isLoggedIn;
Expand All @@ -194,14 +201,6 @@ export default class ComparisonStore {
this.saveAndGoToSession(newSession);
}

readonly _session: MobxPromise<Session>; // must be implemented in subclasses
readonly _originalGroups: MobxPromise<ComparisonGroup[]>; // must be implemented in subclasses
public overlapStrategy: OverlapStrategy; // must be implemented in subclasses
public usePatientLevelEnrichments: boolean; // must be implemented in subclasses
readonly samples: MobxPromise<Sample[]>; // must be implemented in subclass
readonly studies: MobxPromise<CancerStudy[]>; // must be implemented in subclass
// < / >

readonly origin = remoteData({
// the studies that the comparison groups come from
await: () => [this._session],
Expand Down
2 changes: 1 addition & 1 deletion src/shared/lib/createQueryStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function createQueryStore(
// normalize this
query.cancer_study_list =
query.cancer_study_list || query.cancer_study_id;
delete query.cancer_study_id;
delete (query as Partial<CancerStudyQueryUrlParams>).cancer_study_id;

// check if certain parameters should be reset
if (currentQuery) {
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -22642,6 +22642,11 @@ typescript@3.8.3:
resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.8.3.tgz#409eb8544ea0335711205869ec458ab109ee1061"
integrity sha512-MYlEfn5VrLNsgudQTVJeNaQFUAI7DkhnOjdpAp4T+ku1TfQClewlbSuTVHiA+8skNBgaf02TL/kLOvig4y3G8w==

typescript@4.0.3:
version "4.0.3"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.0.3.tgz#153bbd468ef07725c1df9c77e8b453f8d36abba5"
integrity sha512-tEu6DGxGgRJPb/mVPIZ48e69xCn2yRmCgYmDugAVwmJ6o+0u1RI18eO7E7WBTLYLaEVVOhwQmcdhQHweux/WPg==

ua-parser-js@^0.7.18:
version "0.7.19"
resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.19.tgz#94151be4c0a7fb1d001af7022fdaca4642659e4b"
Expand Down