From 266d50e2cd8d38b0305232f296106127a68d2dcc Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Tue, 12 Nov 2024 12:52:54 -0500 Subject: [PATCH 1/6] refact: VariantSearchHeader types + locus search fixes should fix issues when "searching" for genes without having any GFF3 file ingested in the reference genome. --- package-lock.json | 1 + package.json | 1 + src/components/discovery/LocusSearch.js | 44 ++++++---- ...earchHeader.js => VariantSearchHeader.tsx} | 88 +++++++++++++------ src/modules/services/types.ts | 5 +- 5 files changed, 92 insertions(+), 47 deletions(-) rename src/components/discovery/{VariantSearchHeader.js => VariantSearchHeader.tsx} (72%) diff --git a/package-lock.json b/package-lock.json index 523cf35c8..98ecfe074 100644 --- a/package-lock.json +++ b/package-lock.json @@ -49,6 +49,7 @@ "@babel/core": "^7.25.2", "@babel/preset-env": "^7.25.4", "@babel/preset-react": "^7.24.1", + "@types/json-schema": "^7.0.15", "@types/papaparse": "^5.3.14", "@types/react": "^18.3.5", "@types/react-dom": "^18.2.25", diff --git a/package.json b/package.json index 653cacbd5..979f2eee9 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "@babel/core": "^7.25.2", "@babel/preset-env": "^7.25.4", "@babel/preset-react": "^7.24.1", + "@types/json-schema": "^7.0.15", "@types/papaparse": "^5.3.14", "@types/react": "^18.3.5", "@types/react-dom": "^18.2.25", diff --git a/src/components/discovery/LocusSearch.js b/src/components/discovery/LocusSearch.js index 8c8e29215..ff01ad3f3 100644 --- a/src/components/discovery/LocusSearch.js +++ b/src/components/discovery/LocusSearch.js @@ -1,11 +1,15 @@ -import { useCallback, useEffect, useMemo, useState } from "react"; -import { AutoComplete, Tag } from "antd"; +import { useCallback, useEffect, useState } from "react"; +import { AutoComplete, Input, Tag } from "antd"; import PropTypes from "prop-types"; -import { useGeneNameSearch, useReferenceGenomes } from "@/modules/reference/hooks"; +import { useGeneNameSearch } from "@/modules/reference/hooks"; + +// Position notation pattern +// - strip chr prefix, but allow any other types of chromosome - eventually this should instead autocomplete from the +// reference service. +const POS_NOTATION_PATTERN = /(?:CHR|chr)?([\w.-]+):(\d+)-(\d+)/; const parsePosition = (value) => { - const parse = /(?:CHR|chr)([0-9]{1,2}|X|x|Y|y|M|m):(\d+)-(\d+)/; - const result = parse.exec(value); + const result = POS_NOTATION_PATTERN.exec(value); if (!result) { return { chrom: null, start: null, end: null }; @@ -19,16 +23,19 @@ const parsePosition = (value) => { const looksLikePositionNotation = (value) => !value.includes(" ") && value.includes(":"); -const LocusSearch = ({ assemblyId, addVariantSearchValues, handleLocusChange, setLocusValidity }) => { - const referenceGenomes = useReferenceGenomes(); +const LocusSearch = ({ + assemblyId, + geneSearchEnabled, + addVariantSearchValues, + handleLocusChange, + setLocusValidity, +}) => { const [autoCompleteOptions, setAutoCompleteOptions] = useState([]); const [inputValue, setInputValue] = useState(""); - const showAutoCompleteOptions = useMemo( - () => - !!referenceGenomes.itemsByID[assemblyId]?.gff3_gz && inputValue.length && !looksLikePositionNotation(inputValue), - [referenceGenomes, assemblyId, inputValue], - ); + const valueLooksLikePosNot = looksLikePositionNotation(inputValue); + + const showAutoCompleteOptions = geneSearchEnabled && !!inputValue.length && !valueLooksLikePosNot; const handlePositionNotation = useCallback( (value) => { @@ -50,10 +57,10 @@ const LocusSearch = ({ assemblyId, addVariantSearchValues, handleLocusChange, se } }, [inputValue, handlePositionNotation, setLocusValidity]); - const handleChange = useCallback((value) => { - setInputValue(value); - }, []); + const handleChangeInput = useCallback((e) => setInputValue(e.target.value), []); + const handleChangeAutoComplete = useCallback((value) => setInputValue(value), []); + // Don't execute search if showAutoCompleteOptions is false (gene search disabled / input doesn't look like search) const { data: geneSearchResults } = useGeneNameSearch(assemblyId, showAutoCompleteOptions ? inputValue : null); const handleOnBlur = useCallback(() => { @@ -121,10 +128,14 @@ const LocusSearch = ({ assemblyId, addVariantSearchValues, handleLocusChange, se ); }, [geneSearchResults]); + if (!geneSearchEnabled) { + return ; + } + return ( @@ -133,6 +144,7 @@ const LocusSearch = ({ assemblyId, addVariantSearchValues, handleLocusChange, se LocusSearch.propTypes = { assemblyId: PropTypes.string, + geneSearchEnabled: PropTypes.bool, addVariantSearchValues: PropTypes.func, handleLocusChange: PropTypes.func, setLocusValidity: PropTypes.func, diff --git a/src/components/discovery/VariantSearchHeader.js b/src/components/discovery/VariantSearchHeader.tsx similarity index 72% rename from src/components/discovery/VariantSearchHeader.js rename to src/components/discovery/VariantSearchHeader.tsx index 8c65b4b11..b1b3a5a12 100644 --- a/src/components/discovery/VariantSearchHeader.js +++ b/src/components/discovery/VariantSearchHeader.tsx @@ -7,16 +7,27 @@ import LocusSearch from "./LocusSearch"; import { notAlleleCharactersRegex } from "@/utils/misc"; import { useGohanVariantsOverview } from "@/modules/explorer/hooks"; +import type { BentoDataType } from "@/modules/services/types"; import { useAppSelector } from "@/store"; +import type { InputChangeEventHandler } from "@/components/manager/access/types"; +import type { JSONSchema7 } from "json-schema"; +import { useReferenceGenomes } from "@/modules/reference/hooks"; -const isValidLocus = (locus) => locus.chrom !== null && locus.start !== null && locus.end !== null; -const normalizeAlleleText = (text) => text.toUpperCase().replaceAll(notAlleleCharactersRegex, ""); -const containsInvalid = (text) => { +type Locus = { chrom: string | null; start: string | null; end: string | null }; + +const isValidLocus = (locus: Locus) => locus.chrom !== null && locus.start !== null && locus.end !== null; +const normalizeAlleleText = (text: string) => text.toUpperCase().replaceAll(notAlleleCharactersRegex, ""); +const containsInvalid = (text: string) => { const matches = text.toUpperCase().match(notAlleleCharactersRegex); return matches && matches.length > 0; }; -const INITIAL_FIELDS_VALIDITY = { +type FieldsValidity = { + assemblyId: boolean; + locus: boolean; +}; + +const INITIAL_FIELDS_VALIDITY: FieldsValidity = { assemblyId: true, locus: true, }; @@ -25,7 +36,20 @@ const INITIAL_FIELDS_VALIDITY = { const LABEL_COL = { lg: { span: 24 }, xl: { span: 4 }, xxl: { span: 3 } }; const WRAPPER_COL = { lg: { span: 24 }, xl: { span: 20 }, xxl: { span: 18 } }; -const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { +type VariantSearchHeaderProps = { + dataType: BentoDataType; + addVariantSearchValues: ( + x: + | { assemblyId: string } + | { alt: string } + | { ref: string } + | { + genotypeType: string; + }, + ) => void; +}; + +const VariantSearchHeader = ({ dataType, addVariantSearchValues }: VariantSearchHeaderProps) => { const { data: variantsOverviewResults, isFetching: isFetchingVariantsOverview } = useGohanVariantsOverview(); const overviewAssemblyIds = useMemo(() => { const hasAssemblyIds = @@ -40,26 +64,34 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { const [refFormReceivedValidKeystroke, setRefFormReceivedValidKeystroke] = useState(true); const [altFormReceivedValidKeystroke, setAltFormReceivedValidKeystroke] = useState(true); - const [activeRefValue, setActiveRefValue] = useState(null); - const [activeAltValue, setActiveAltValue] = useState(null); - const [assemblyId, setAssemblyId] = useState(overviewAssemblyIds.length === 1 ? overviewAssemblyIds[0] : null); - const [locus, setLocus] = useState({ chrom: null, start: null, end: null }); + const [activeRefValue, setActiveRefValue] = useState(""); + const [activeAltValue, setActiveAltValue] = useState(""); + + const [assemblyId, setAssemblyId] = useState( + overviewAssemblyIds.length === 1 ? overviewAssemblyIds[0] : null, + ); + const referenceGenomes = useReferenceGenomes(); + const geneSearchEnabled = assemblyId !== null && !!referenceGenomes.itemsByID[assemblyId]?.gff3_gz; + + const [locus, setLocus] = useState({ chrom: null, start: null, end: null }); const { isSubmittingSearch: isSubmitting } = useAppSelector((state) => state.explorer); // begin with required fields considered valid, so user isn't assaulted with error messages - const [fieldsValidity, setFieldsValidity] = useState(INITIAL_FIELDS_VALIDITY); + const [fieldsValidity, setFieldsValidity] = useState(INITIAL_FIELDS_VALIDITY); - const genotypeSchema = dataType.schema?.properties?.calls?.items?.properties?.genotype_type; + const genotypeSchema = ( + (dataType.schema?.properties?.calls as JSONSchema7 | undefined)?.items as JSONSchema7 | undefined + )?.properties?.genotype_type as JSONSchema7 | undefined; const genotypeSchemaDescription = genotypeSchema?.description; const genotypeOptions = useMemo( - () => (genotypeSchema?.enum ?? []).map((value) => ({ value, label: value })), + () => ((genotypeSchema?.enum ?? []) as string[]).map((value: string) => ({ value, label: value })), [genotypeSchema], ); const helpText = useMemo(() => { - const assemblySchema = dataType.schema?.properties?.assembly_id; + const assemblySchema = dataType.schema?.properties?.assembly_id as JSONSchema7 | undefined; return { - assemblyId: assemblySchema?.description, + assemblyId: assemblySchema?.description ?? "", genotype: genotypeSchemaDescription, // eslint-disable-next-line quotes locus: 'Enter gene name (eg "BRCA1") or position ("chr17:41195311-41278381")', @@ -74,16 +106,16 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { // check assembly if (!assemblyId) { // change assemblyId help text & outline - setFieldsValidity({ ...fieldsValidity, assemblyId: false }); + setFieldsValidity((fv) => ({ ...fv, assemblyId: false })); } // check locus const { chrom, start, end } = locus; if (!chrom || !start || !end) { // change locus help text & outline - setFieldsValidity({ ...fieldsValidity, locus: false }); + setFieldsValidity((fv) => ({ ...fv, locus: false })); } - }, [assemblyId, locus, fieldsValidity]); + }, [assemblyId, locus]); useEffect(() => { if (isSubmitting) { @@ -91,15 +123,12 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { } }, [isSubmitting, validateVariantSearchForm]); - const setLocusValidity = useCallback( - (isValid) => { - setFieldsValidity({ ...fieldsValidity, locus: isValid }); - }, - [fieldsValidity], - ); + const setLocusValidity = useCallback((isValid: boolean) => { + setFieldsValidity((fv) => ({ ...fv, locus: isValid })); + }, []); const handleLocusChange = useCallback( - (locus) => { + (locus: Locus) => { setLocusValidity(isValidLocus(locus)); // set even if invalid, so we don't keep old values @@ -109,7 +138,7 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { ); const handleAssemblyIdChange = useCallback( - (value) => { + (value: string) => { addVariantSearchValues({ assemblyId: value }); setAssemblyId(value); }, @@ -117,13 +146,13 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { ); const handleGenotypeChange = useCallback( - (value) => { + (value: string) => { addVariantSearchValues({ genotypeType: value }); }, [addVariantSearchValues], ); - const handleRefChange = useCallback( + const handleRefChange = useCallback( (e) => { const latestInputValue = e.target.value; const normalizedRef = normalizeAlleleText(latestInputValue); @@ -141,7 +170,7 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { [addVariantSearchValues], ); - const handleAltChange = useCallback( + const handleAltChange = useCallback( (e) => { const latestInputValue = e.target.value; const normalizedAlt = normalizeAlleleText(latestInputValue); @@ -187,13 +216,14 @@ const VariantSearchHeader = ({ dataType, addVariantSearchValues }) => { Date: Thu, 14 Nov 2024 10:09:27 -0500 Subject: [PATCH 2/6] fix: try to address redux state/variant input mismatch --- src/components/discovery/LocusSearch.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/components/discovery/LocusSearch.js b/src/components/discovery/LocusSearch.js index ff01ad3f3..db1f48d62 100644 --- a/src/components/discovery/LocusSearch.js +++ b/src/components/discovery/LocusSearch.js @@ -128,6 +128,11 @@ const LocusSearch = ({ ); }, [geneSearchResults]); + useEffect(() => { + // If the input mode changes, we need to clear the corresponding Redux state since it isn't directly linked + addVariantSearchValues({ chrom: null, start: null, end: null }); + }, [addVariantSearchValues, geneSearchEnabled]); + if (!geneSearchEnabled) { return ; } From 758fdc68fe9332cc5f342b6eaf52d3189fe91480 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Thu, 14 Nov 2024 11:23:30 -0500 Subject: [PATCH 3/6] hmm --- src/components/discovery/LocusSearch.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/discovery/LocusSearch.js b/src/components/discovery/LocusSearch.js index db1f48d62..276cd5379 100644 --- a/src/components/discovery/LocusSearch.js +++ b/src/components/discovery/LocusSearch.js @@ -130,6 +130,7 @@ const LocusSearch = ({ useEffect(() => { // If the input mode changes, we need to clear the corresponding Redux state since it isn't directly linked + handleLocusChange({ chrom: null, start: null, end: null }); addVariantSearchValues({ chrom: null, start: null, end: null }); }, [addVariantSearchValues, geneSearchEnabled]); From e2da511612ac4fabfca7dcdaa6c3566ea1ce1d1f Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Thu, 14 Nov 2024 11:25:43 -0500 Subject: [PATCH 4/6] revert --- src/components/discovery/LocusSearch.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/discovery/LocusSearch.js b/src/components/discovery/LocusSearch.js index 276cd5379..db1f48d62 100644 --- a/src/components/discovery/LocusSearch.js +++ b/src/components/discovery/LocusSearch.js @@ -130,7 +130,6 @@ const LocusSearch = ({ useEffect(() => { // If the input mode changes, we need to clear the corresponding Redux state since it isn't directly linked - handleLocusChange({ chrom: null, start: null, end: null }); addVariantSearchValues({ chrom: null, start: null, end: null }); }, [addVariantSearchValues, geneSearchEnabled]); From 154f5628b58f5ea7eca784f1753fa509d5afaa5c Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 15 Nov 2024 12:44:32 -0500 Subject: [PATCH 5/6] lint: imports --- src/components/discovery/VariantSearchHeader.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/discovery/VariantSearchHeader.tsx b/src/components/discovery/VariantSearchHeader.tsx index b1b3a5a12..4d88177d3 100644 --- a/src/components/discovery/VariantSearchHeader.tsx +++ b/src/components/discovery/VariantSearchHeader.tsx @@ -1,17 +1,17 @@ import { useState, useEffect, useCallback, useMemo } from "react"; import PropTypes from "prop-types"; +import type { JSONSchema7 } from "json-schema"; import { Form, Input, Select } from "antd"; import LocusSearch from "./LocusSearch"; -import { notAlleleCharactersRegex } from "@/utils/misc"; +import type { InputChangeEventHandler } from "@/components/manager/access/types"; import { useGohanVariantsOverview } from "@/modules/explorer/hooks"; import type { BentoDataType } from "@/modules/services/types"; -import { useAppSelector } from "@/store"; -import type { InputChangeEventHandler } from "@/components/manager/access/types"; -import type { JSONSchema7 } from "json-schema"; import { useReferenceGenomes } from "@/modules/reference/hooks"; +import { useAppSelector } from "@/store"; +import { notAlleleCharactersRegex } from "@/utils/misc"; type Locus = { chrom: string | null; start: string | null; end: string | null }; From 5e9a9dcc3d84bbaf820b4bba9f8660f0ed260fd5 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 15 Nov 2024 12:48:51 -0500 Subject: [PATCH 6/6] chore: try to address review comments... --- src/components/discovery/LocusSearch.js | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/components/discovery/LocusSearch.js b/src/components/discovery/LocusSearch.js index db1f48d62..fbce8c65d 100644 --- a/src/components/discovery/LocusSearch.js +++ b/src/components/discovery/LocusSearch.js @@ -1,8 +1,10 @@ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { AutoComplete, Input, Tag } from "antd"; import PropTypes from "prop-types"; import { useGeneNameSearch } from "@/modules/reference/hooks"; +const NULL_LOCUS = { chrom: null, start: null, end: null }; + // Position notation pattern // - strip chr prefix, but allow any other types of chromosome - eventually this should instead autocomplete from the // reference service. @@ -12,7 +14,7 @@ const parsePosition = (value) => { const result = POS_NOTATION_PATTERN.exec(value); if (!result) { - return { chrom: null, start: null, end: null }; + return NULL_LOCUS; } const chrom = result[1].toUpperCase(); //for eg 'x', has no effect on numbers @@ -30,6 +32,8 @@ const LocusSearch = ({ handleLocusChange, setLocusValidity, }) => { + const mounted = useRef(false); + const [autoCompleteOptions, setAutoCompleteOptions] = useState([]); const [inputValue, setInputValue] = useState(""); @@ -77,8 +81,8 @@ const LocusSearch = ({ const isPositionNotation = inputValue.includes(":") && !isAutoCompleteOption; if (!(isAutoCompleteOption || isPositionNotation)) { - handleLocusChange({ chrom: null, start: null, end: null }); - addVariantSearchValues({ chrom: null, start: null, end: null }); + handleLocusChange(NULL_LOCUS); + addVariantSearchValues(NULL_LOCUS); return; } @@ -130,8 +134,16 @@ const LocusSearch = ({ useEffect(() => { // If the input mode changes, we need to clear the corresponding Redux state since it isn't directly linked - addVariantSearchValues({ chrom: null, start: null, end: null }); - }, [addVariantSearchValues, geneSearchEnabled]); + // - if we're making the state newly invalid (rather than on first run), run handleLocusChange() too + if (mounted.current) handleLocusChange(NULL_LOCUS); + addVariantSearchValues(NULL_LOCUS); + }, [addVariantSearchValues, handleLocusChange, geneSearchEnabled]); + + // This effect needs to be last before rendering! + // A small hack to change the above effect's behaviour if we're making the input invalid (vs. it starting invalid) + useEffect(() => { + mounted.current = true; + }, []); if (!geneSearchEnabled) { return ;