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.

Sign up for GitHub

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

Jump to bottom

Fix ClinVar track crash #1532

Merged
merged 2 commits into from
May 23, 2024
Merged

Fix ClinVar track crash #1532

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90c654e
fix(browser): clinvar track doesn't crash with unknown transcript fra…
rileyhgrant May 1, 2024
0c82443
fixup(browser): include suggestions from code review
rileyhgrant May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions browser/src/ClinvarVariantsTrack/ClinvarAllVariantsPlot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,21 +210,27 @@ const VariantLine = ({
}

if (category === 'frameshift') {
const transcript = transcripts.find((t) => t.transcript_id === variant.transcript_id)
const transcript: Transcript | undefined = transcripts.find(
(t) => t.transcript_id === variant.transcript_id
)
const [endpoint1, endpoint2] = getGlobalFrameshiftCoordinates(variant, transcript)
const frameshiftMinPos = Math.min(endpoint1, endpoint2)
const frameshiftMaxPos = Math.max(endpoint1, endpoint2)
const terminationPos =
transcript && transcript.strand === '+' ? frameshiftMaxPos : frameshiftMinPos
// @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'.
const frameshiftExonRegions = transcript.exons
.sort((e1, e2) => e1.start - e2.start)
.filter((e) => e.start <= frameshiftMaxPos && e.stop >= frameshiftMinPos)
.map((e) => ({
start: Math.max(e.start, frameshiftMinPos),
stop: Math.min(e.stop, frameshiftMaxPos),
feature_type: e.feature_type,
}))

// if a frameshift variant-consequence pair from ClinVar exists for a transcript
Copy link
Contributor

Choose a reason for hiding this comment

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

So this fix is fine far as it goes, but I'd suggest you also do two other things:

  1. add a type annotation a couple lines above here reflecting that transcript might be undefined, i.e. change line 213 to something like: const transcript: Transcript | undefined = .... That way we won't get bitten here by a similar case in the future.
  2. Update getGlobalFrameshiftCoordinates also to reflect that transcript might be undefined, the logic might be fine as-is but a type annotation on there would be good in any case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha very good call, I didn't think to add a type annotation on the variable itself.

I added a fixup commit to address point 1.

Point 2. is already addressed before this PR (getGlobalFrameshift... expects the possibility of undefined and handles it if so), so that's why theres not fixup commit to address that.

// that the browser doesn't recognize, use an empty array for exon information
const frameshiftExonRegions = transcript
? transcript.exons
.sort((e1, e2) => e1.start - e2.start)
.filter((e) => e.start <= frameshiftMaxPos && e.stop >= frameshiftMinPos)
.map((e) => ({
start: Math.max(e.start, frameshiftMinPos),
stop: Math.min(e.stop, frameshiftMaxPos),
feature_type: e.feature_type,
}))
: []

return (
<TooltipAnchor
Expand Down
Loading