From 8feb5f99006a9fa3d47932c28b930c83be900f4d Mon Sep 17 00:00:00 2001 From: Lucas <97296331+lucas-paulger-sonarsource@users.noreply.github.com> Date: Wed, 31 Jul 2024 15:18:19 +0300 Subject: SONAR-22646 Improve issue indicator render cycle (#11448) --- .../src/sonar-aligned/hljs/HljsUnderlinePlugin.ts | 2 +- .../design-system/src/sonar-aligned/hljs/index.ts | 1 + .../SourceViewer/SourceViewerPreview.tsx | 243 ++++++++++++--------- 3 files changed, 147 insertions(+), 99 deletions(-) diff --git a/server/sonar-web/design-system/src/sonar-aligned/hljs/HljsUnderlinePlugin.ts b/server/sonar-web/design-system/src/sonar-aligned/hljs/HljsUnderlinePlugin.ts index e1aec179d9a..8bbead1f742 100644 --- a/server/sonar-web/design-system/src/sonar-aligned/hljs/HljsUnderlinePlugin.ts +++ b/server/sonar-web/design-system/src/sonar-aligned/hljs/HljsUnderlinePlugin.ts @@ -19,7 +19,7 @@ */ import { HighlightResult } from 'highlight.js'; -interface UnderlineRangePosition { +export interface UnderlineRangePosition { cursorOffset: number; line: number; } diff --git a/server/sonar-web/design-system/src/sonar-aligned/hljs/index.ts b/server/sonar-web/design-system/src/sonar-aligned/hljs/index.ts index 0816564955e..a0279b93e76 100644 --- a/server/sonar-web/design-system/src/sonar-aligned/hljs/index.ts +++ b/server/sonar-web/design-system/src/sonar-aligned/hljs/index.ts @@ -20,3 +20,4 @@ export { hljsIssueIndicatorPlugin } from './HljsIssueIndicatorPlugin'; export { hljsUnderlinePlugin } from './HljsUnderlinePlugin'; +export type { UnderlineRangePosition } from './HljsUnderlinePlugin'; diff --git a/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerPreview.tsx b/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerPreview.tsx index e56b792194e..4cece1cc978 100644 --- a/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerPreview.tsx +++ b/server/sonar-web/src/main/js/components/SourceViewer/SourceViewerPreview.tsx @@ -20,9 +20,16 @@ import { ICell, isCode, isMarkdown } from '@jupyterlab/nbformat'; import { Spinner } from '@sonarsource/echoes-react'; -import { FlagMessage, hljsIssueIndicatorPlugin, hljsUnderlinePlugin } from 'design-system'; -import React, { useEffect, useMemo, useState } from 'react'; +import { + FlagMessage, + hljsIssueIndicatorPlugin, + hljsUnderlinePlugin, + UnderlineRangePosition, +} from 'design-system'; + +import React, { forwardRef, useEffect, useMemo, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; +import { To } from 'react-router-dom'; import { useLocation, useRouter } from '~sonar-aligned/components/hoc/withRouter'; import { JupyterCodeCell, @@ -48,22 +55,35 @@ export interface Props { } type IssuesByCell = { [key: number]: IssuesByLine }; +type IssueByLine = { + end: UnderlineRangePosition; + issue: Issue; + start: UnderlineRangePosition; +}; type IssuesByLine = { - [line: number]: { - end: { cursorOffset: number; line: number }; - issue: Issue; - start: { cursorOffset: number; line: number }; - }[]; + [line: number]: IssueByLine[]; }; type IssueKeysByLine = { [line: number]: string[] }; +type IssueIndicatorsProps = { + branchLike: BranchLike; + component: Component; + issuesByCell: IssuesByCell; + jupyterRef: React.RefObject; +}; -const DELAY_FOR_PORTAL_INDEX_ELEMENT = 200; +type IssueMapper = { + issueUrl: To; + key: string; + lineIndex: number; + onlyIssues: Issue[]; +}; export default function SourceViewerPreview(props: Readonly) { const { component, branchLike } = props; const [issues, setIssues] = useState([]); const [issuesByCell, setIssuesByCell] = useState({}); - const [renderedCells, setRenderedCells] = useState([]); + const jupyterNotebookRef = useRef(null); + const { data, isLoading } = useRawSourceQuery({ key: component, ...getBranchLikeQuery(branchLike), @@ -81,8 +101,6 @@ export default function SourceViewerPreview(props: Readonly) { } }, [data]); - const [hasRendered, setHasRendered] = useState(false); - useEffect(() => { const fetchData = async () => { const issues = await loadIssues(component, branchLike); @@ -122,12 +140,10 @@ export default function SourceViewerPreview(props: Readonly) { const endOffset = pathToCursorInCell(mapper.get(end)); if (!startOffset || !endOffset) { - setRenderedCells(null); return; } if (startOffset.cell !== endOffset.cell) { - setRenderedCells(null); return; } @@ -151,7 +167,6 @@ export default function SourceViewerPreview(props: Readonly) { } }); - setRenderedCells(jupyterNotebook?.cells); setIssuesByCell(newIssuesByCell); }, [issues, data, jupyterNotebook]); @@ -167,7 +182,7 @@ export default function SourceViewerPreview(props: Readonly) { ); } - if (!renderedCells) { + if (!jupyterNotebook?.cells) { return ( {translate('source_viewer.jupyter.preview.error')} @@ -178,15 +193,16 @@ export default function SourceViewerPreview(props: Readonly) { return ( <> setHasRendered(true)} + ref={jupyterNotebookRef} /> - {hasRendered && issues && componentContext && branchLike && ( + {issues && componentContext && branchLike && ( )} @@ -196,7 +212,6 @@ export default function SourceViewerPreview(props: Readonly) { type JupyterNotebookProps = { cells: ICell[]; issuesByCell: IssuesByCell; - onRender: () => void; }; function mapIssuesToIssueKeys(issuesByLine: IssuesByLine): IssueKeysByLine { @@ -206,95 +221,127 @@ function mapIssuesToIssueKeys(issuesByLine: IssuesByLine): IssueKeysByLine { }, {} as IssueKeysByLine); } -function RenderJupyterNotebook({ cells, issuesByCell, onRender }: Readonly) { - useEffect(() => { - // the `issue-key-${issue.key}` need to be rendered before we trigger the IssueIndicators below - setTimeout(onRender, DELAY_FOR_PORTAL_INDEX_ELEMENT); - }, [onRender]); - - const buildCellsBlocks = useMemo(() => { - return cells.map((cell: ICell, index: number) => { - let sourceLines = Array.isArray(cell.source) ? cell.source : [cell.source]; - const issuesByLine = issuesByCell[index]; - if (!issuesByLine) { +const RenderJupyterNotebook = forwardRef( + ({ cells, issuesByCell }, ref) => { + const buildCellsBlocks = useMemo(() => { + return cells.map((cell: ICell, index: number) => { + let sourceLines = Array.isArray(cell.source) ? cell.source : [cell.source]; + const issuesByLine = issuesByCell[index]; + if (!issuesByLine) { + return { + cell, + sourceLines, + }; + } + const issues = mapIssuesToIssueKeys(issuesByLine); + const flatIssues = Object.entries(issuesByLine).flatMap(([, issues]) => issues); + + sourceLines = hljsUnderlinePlugin.tokenize(sourceLines, flatIssues); + sourceLines = hljsIssueIndicatorPlugin.addIssuesToLines(sourceLines, issues); + return { cell, sourceLines, }; - } - const issues = mapIssuesToIssueKeys(issuesByLine); - const flatIssues = Object.entries(issuesByLine).flatMap(([, issues]) => issues); - - sourceLines = hljsUnderlinePlugin.tokenize(sourceLines, flatIssues); - sourceLines = hljsIssueIndicatorPlugin.addIssuesToLines(sourceLines, issues); + }); + }, [cells, issuesByCell]); - return { - cell, - sourceLines, - }; - }); - }, [cells, issuesByCell]); - - return ( - <> - {buildCellsBlocks.map((element, index) => { - const { cell, sourceLines } = element; - if (isCode(cell)) { - return ( - - ); - } else if (isMarkdown(cell)) { - return ; - } - return null; - })} - - ); -} + return ( +
+ {buildCellsBlocks.map((element, index) => { + const { cell, sourceLines } = element; + if (isCode(cell)) { + return ( + + ); + } else if (isMarkdown(cell)) { + return ; + } + return null; + })} +
+ ); + }, +); -type IssueIndicatorsProps = { - branchLike: BranchLike; - component: Component; - issuesByCell: IssuesByCell; -}; +RenderJupyterNotebook.displayName = 'RenderJupyterNotebook'; -function IssueIndicators({ issuesByCell, component, branchLike }: Readonly) { +function IssueIndicators({ + issuesByCell, + component, + branchLike, + jupyterRef, +}: Readonly) { const location = useLocation(); const query = parseQuery(location.query); + const onlyIssuesMap = (issues: IssueByLine[]) => issues.map(({ issue }) => issue); + const mappedIssues = useMemo(() => { + return Object.entries(issuesByCell).flatMap(([, issuesByLine]) => + Object.entries(issuesByLine).map(([, issues]) => { + const firstIssue = issues[0].issue; + const onlyIssues = onlyIssuesMap(issues); + const urlQuery = { + ...getBranchLikeQuery(branchLike), + ...serializeQuery(query), + open: firstIssue.key, + }; + const issueUrl = component?.key + ? getComponentIssuesUrl(component?.key, urlQuery) + : getIssuesUrl(urlQuery); + return { key: firstIssue.key, issueUrl, onlyIssues, lineIndex: issues[0].start.line }; + }), + ); + // we only need to recompute this with new issues + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [issuesByCell]); + + return mappedIssues.map((mappedIssues) => ( + + )); +} + +function PortalLineIssuesIndicator(props: { + issueMapper: IssueMapper; + jupyterRef: React.RefObject; +}) { + const { jupyterRef, issueMapper } = props; const router = useRouter(); - const issuePortals = Object.entries(issuesByCell).flatMap(([, issuesByLine]) => - Object.entries(issuesByLine).map(([lineIndex, issues]) => { - const firstIssue = issues[0].issue; - const onlyIssues = issues.map(({ issue }) => issue); - const urlQuery = { - ...getBranchLikeQuery(branchLike), - ...serializeQuery(query), - open: firstIssue.key, - }; - const issueUrl = component?.key - ? getComponentIssuesUrl(component?.key, urlQuery) - : getIssuesUrl(urlQuery); - const portalIndexElement = document.getElementById(`issue-key-${firstIssue.key}`); - return portalIndexElement ? ( - - {createPortal( - router.navigate(issueUrl)} - line={{ line: Number(lineIndex) }} - as="span" - />, - portalIndexElement, - )} - - ) : null; - }), - ); + const [mutationCount, setMutationCount] = useState(0); + + useEffect(() => { + if (!jupyterRef.current) { + return; + } + setMutationCount((count) => count + 1); + }, [jupyterRef]); - return <>{issuePortals}; + const { key, lineIndex, onlyIssues, issueUrl } = issueMapper; + const element = document.getElementById(`issue-key-${key}`); + + // we don't have the jupyterRef yet + if (mutationCount === 0) { + return null; + } + + if (!element) { + return null; + } + return createPortal( + router.navigate(issueUrl)} + line={{ line: Number(lineIndex) }} + as="span" + />, + element, + ); } -- cgit v1.2.3 href='#n76'>76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124