Skip to content

Commit

Permalink
Fix error-prone code exposed by TypeScript conversion
Browse files Browse the repository at this point in the history 
These "worked" in normal JS under expected usage but raise errors by the
TypeScript compiler which generally catches unhandled edge cases. Each
has been addressed with inline reasoning.
  • Loading branch information
@victorlin
victorlin committed Jun 4, 2024
1 parent 68136b2 commit 795558d
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 35 deletions.
46 changes: 32 additions & 14 deletions static-site/src/components/ListResources/IndividualResource.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ const iconWidth = 20; // not including text
const gapSize = 10;
export const getMaxResourceWidth = (displayResources: Resource[]) => {
return displayResources.reduce((w, r) => {
if (!r.displayName || !r.updateCadence) return w

/* add the pixels for the display name */
let _w = r.displayName.default.length * namePxPerChar;
if (r.nVersions) {
_w += gapSize + iconWidth;
_w += ((r?.updateCadence?.summary?.length || 0) + 5 + String(r.nVersions).length)*summaryPxPerChar;
_w += ((r.updateCadence.summary.length || 0) + 5 + String(r.nVersions).length)*summaryPxPerChar;
}
return _w>w ? _w : w;
}, 200); // 200 (pixels) is the minimum
Expand Down Expand Up @@ -116,35 +118,51 @@ export const IndividualResource = ({resource, isMobile}: IndividualResourceProps
const ref = useRef<HTMLDivElement>(null);
const [topOfColumn, setTopOfColumn] = useState(false);
useEffect(() => {
// don't do anything if the ref is undefined or the parent is not a div (IndividualResourceContainer)
if (!ref.current
|| !ref.current.parentNode
|| ref.current.parentNode.nodeName != 'DIV') return;

/* The column CSS is great but doesn't allow us to know if an element is at
the top of its column, so we resort to JS */
if (ref.current.offsetTop===ref.current.parentNode.offsetTop) {
if (ref.current.offsetTop===(ref.current.parentNode as HTMLDivElement).offsetTop) {
setTopOfColumn(true);
}
}, []);

// don't show anything if display name is unavailable
if (!resource.displayName) return null

// add history if mobile and resource is versioned
let history: React.JSX.Element | null = null
if (!isMobile && resource.versioned && resource.updateCadence && resource.nVersions) {
history = (
<TooltipWrapper description={resource.updateCadence.description +
`<br/>Last known update on ${resource.lastUpdated}` +
`<br/>${resource.nVersions} snapshots of this dataset available (click to see them)`}>
<IconContainer
Icon={MdHistory}
text={`${resource.updateCadence.summary} (n=${resource.nVersions})`}
// if setModalResource is null, click does nothing
handleClick={() => setModalResource?.(resource)}
/>
</TooltipWrapper>
)
}

return (
<Container ref={ref}>

<FlexRow>

<TooltipWrapper description={`Last known update on ${resource.lastUpdated}`}>
<ResourceLinkWrapper onShiftClick={() => setModalResource(resource)}>
{/* if setModalResource is null, shift-click does nothing */}
<ResourceLinkWrapper onShiftClick={() => setModalResource?.(resource)}>
<Name displayName={resource.displayName} href={resource.url} topOfColumn={topOfColumn}/>
</ResourceLinkWrapper>
</TooltipWrapper>

{resource.versioned && !isMobile && (
<TooltipWrapper description={resource.updateCadence.description +
`<br/>Last known update on ${resource.lastUpdated}` +
`<br/>${resource.nVersions} snapshots of this dataset available (click to see them)`}>
<IconContainer
Icon={MdHistory}
text={`${resource.updateCadence.summary} (n=${resource.nVersions})`}
handleClick={() => setModalResource(resource)}
/>
</TooltipWrapper>
)}
{history}

</FlexRow>

Expand Down
14 changes: 10 additions & 4 deletions static-site/src/components/ListResources/Modal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export const ResourceModal = ({resource, dismissModal}: ResourceModalProps) => {
_draw(ref, resource)
}, [ref, resource])

if (!resource) return null;
// modal is only applicable for versioned resources
if (!resource || !resource.dates || !resource.updateCadence) return null;

const summary = _snapshotSummary(resource.dates);
return (
Expand Down Expand Up @@ -132,8 +133,9 @@ const Title = styled.div`

function _snapshotSummary(dates: string[]) {
const d = [...dates].sort()
const d1 = new Date(d.at( 0)).getTime();
const d2 = new Date(d.at(-1)).getTime();
// assume that dates has at least one entry
const d1 = new Date(d.at( 0)!).getTime();
const d2 = new Date(d.at(-1)!).getTime();
const days = (d2 - d1)/1000/60/60/24;
let duration = '';
if (days < 100) duration=`${days} days`;
Expand All @@ -143,6 +145,9 @@ function _snapshotSummary(dates: string[]) {
}

function _draw(ref, resource: Resource) {
// do nothing if resource has no dates
if (!resource.dates) return

/* Note that _page_ resizes by themselves will not result in this function
rerunning, which isn't great, but for a modal I think it's perfectly
acceptable */
Expand Down Expand Up @@ -172,7 +177,8 @@ function _draw(ref, resource: Resource) {

/* Create the x-scale and draw the x-axis */
const x = d3.scaleTime()
.domain([flatData[0].date, new Date()]) // the domain extends to the present day
// presence of dates on resource has already been checked so this assertion is safe
.domain([flatData[0]!.date, new Date()]) // the domain extends to the present day
.range([graphIndent, width-graphIndent])
svg.append('g')
.attr("transform", `translate(0, ${heights.height - heights.marginBelowAxis})`)
Expand Down
5 changes: 3 additions & 2 deletions static-site/src/components/ListResources/ResourceGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ const ResourceGroupHeader = ({group, isMobile, setCollapsed, collapsible, isColl
)}
{quickLinksToDisplay.map((ql) => (
<div style={{paddingLeft: '5px'}} key={ql.name}>
<ResourceLinkWrapper onShiftClick={() => {setModalResource(resourcesByName[ql.name])}}>
{/* if setModalResource is null, shift-click does nothing */}
<ResourceLinkWrapper onShiftClick={() => {setModalResource?.(resourcesByName[ql.name])}}>
<ResourceLink href={`/${ql.name}`} target="_blank" rel="noreferrer">
{ql.display}
</ResourceLink>
Expand Down Expand Up @@ -252,7 +253,7 @@ function _setDisplayName(resources: Resource[]) {
if (i===0) {
name = r.nameParts.join(sep);
} else {
let matchIdx = r.nameParts.map((word, j) => word === resources[i-1].nameParts[j]).findIndex((v) => !v);
let matchIdx = r.nameParts.map((word, j) => word === resources[i-1]?.nameParts[j]).findIndex((v) => !v);
if (matchIdx===-1) { // -1 means every word is in the preceding name, but we should display the last word anyway
matchIdx = r.nameParts.length-2;
}
Expand Down
4 changes: 3 additions & 1 deletion static-site/src/components/ListResources/Showcase.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ const CardOuter = styled.div`

const themeColors = [...theme.titleColors];
const getColor = () => {
themeColors.push(themeColors.shift());
// rotate colors by moving the first color (which is always defined) to the end
themeColors.push(themeColors.shift()!);
// return the last color
return themeColors.at(-1);
}

Expand Down
10 changes: 8 additions & 2 deletions static-site/src/components/ListResources/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,15 @@ function ListResourcesResponsive(props: ListResourcesResponsiveProps) {
const [elWidth, setElWidth] = useState<number>(0);
useEffect(() => {
const observer = new ResizeObserver(([entry]) => {
setElWidth(entry.contentRect.width);
if (entry) {
// don't do anything if entry is undefined
setElWidth(entry.contentRect.width);
}
});
observer.observe(ref.current);
if (ref.current) {
// don't do anything if ref is undefined
observer.observe(ref.current);
}
return () => {
observer.disconnect();
};
Expand Down
30 changes: 18 additions & 12 deletions static-site/src/components/ListResources/useDataFetch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,35 @@ interface Partitions {
*/
function partitionByPathogen(pathVersions: PathVersions, pathPrefix: string, versioned: boolean) {
return Object.keys(pathVersions).reduce((store: Partitions, name: string) => {
const nameParts = name.split('/');
const sortedDates = [...pathVersions[name]].sort();
// name will always be a valid key based on reduce() above
const sortedDates = [...pathVersions[name]!].sort();

// do nothing if resource has no dates
if (sortedDates.length < 1) return store

let groupName = nameParts[0];
const nameParts = name.split('/');
// split() will always return at least 1 string
let groupName = nameParts[0]!;

if (!store[groupName]) store[groupName] = []
const resourceDetails: Resource = {
name,
groupName, /* decoupled from nameParts */
nameParts,
sortingName: _sortableName(nameParts),
url: `/${pathPrefix}${name}`,
lastUpdated: sortedDates.at(-1),
lastUpdated: sortedDates.at(-1)!,
versioned
};
if (versioned) {
resourceDetails.firstUpdated = sortedDates[0];
resourceDetails.lastUpdated = sortedDates.at(-1);
resourceDetails.firstUpdated = sortedDates[0]!;
resourceDetails.lastUpdated = sortedDates.at(-1)!;
resourceDetails.dates = sortedDates;
resourceDetails.nVersions = sortedDates.length;
resourceDetails.updateCadence = updateCadence(sortedDates.map((date)=> new Date(date)));
}

store[groupName].push(resourceDetails)
if (!store[groupName]) store[groupName] = [];
store[groupName]!.push(resourceDetails)

return store;
}, {});
Expand All @@ -100,12 +105,13 @@ function partitionByPathogen(pathVersions: PathVersions, pathPrefix: string, ver
*/
function groupsFrom(partitions: Partitions, pathPrefix: string, defaultGroupLinks: boolean, groupDisplayNames: GroupDisplayNames) {
return Object.keys(partitions).map(groupName => {
const resources = partitions[groupName];
// groupName will always be a valid key based on map() above
const resources = partitions[groupName]!;
const groupInfo: Group = {
groupName: groupName,
nResources: resources.length,
nVersions: resources.reduce((total, r) => r.nVersions ? total+r.nVersions : total, 0) || undefined,
lastUpdated: resources.map((r) => r.lastUpdated).sort().at(-1),
lastUpdated: resources.map((r) => r.lastUpdated).sort().at(-1)!,
resources,
}
/* add optional properties */
Expand All @@ -128,7 +134,7 @@ function groupsFrom(partitions: Partitions, pathPrefix: string, defaultGroupLink
function _sortableName(words: string[]) {
const w = words.map((word) => {
const m = word.match(/^(\d+)([ym])$/);
if (m) {
if (m && m[1]) {
if (m[2]==='y') return String(parseInt(m[1])*12).padStart(4,'0')
return m[1].padStart(4,'0')
}
Expand Down Expand Up @@ -163,7 +169,7 @@ function updateCadence(dateObjects) {
return {summary: "rarely", description: `This dataset has been updated ${intervals.length+1} times in the past 2 years.`};
}

const lastUpdateDaysAgo = Math.round(((new Date()) - dateObjects.at(-1))/msInADay);
const lastUpdateDaysAgo = Math.round(((new Date()).getTime() - dateObjects.at(-1))/msInADay);
const median = intervals[Math.floor(intervals.length/2)];
const mad = intervals.map((x) => Math.abs(x-median)).sort((a, b) => a-b)[Math.floor(intervals.length/2)]

Expand Down

0 comments on commit 795558d

Please sign in to comment.