diff --git a/packages/html-reporter/src/headerView.tsx b/packages/html-reporter/src/headerView.tsx index f2ae20ce01..2e2a47ace1 100644 --- a/packages/html-reporter/src/headerView.tsx +++ b/packages/html-reporter/src/headerView.tsx @@ -37,7 +37,7 @@ export const HeaderView: React.FC
diff --git a/packages/recorder/src/callLog.tsx b/packages/recorder/src/callLog.tsx index a40be416dc..022ffa75df 100644 --- a/packages/recorder/src/callLog.tsx +++ b/packages/recorder/src/callLog.tsx @@ -30,7 +30,7 @@ export const CallLogView: React.FC = ({ language, log, }) => { - const messagesEndRef = React.createRef(); + const messagesEndRef = React.useRef(null); const [expandOverrides, setExpandOverrides] = React.useState>(new Map()); React.useLayoutEffect(() => { if (log.find(callLog => callLog.reveal)) diff --git a/packages/recorder/src/recorder.tsx b/packages/recorder/src/recorder.tsx index ec51aa5537..da5d628706 100644 --- a/packages/recorder/src/recorder.tsx +++ b/packages/recorder/src/recorder.tsx @@ -78,7 +78,7 @@ export const Recorder: React.FC = ({ setFileId(value); }; - const messagesEndRef = React.createRef(); + const messagesEndRef = React.useRef(null); React.useLayoutEffect(() => { messagesEndRef.current?.scrollIntoView({ block: 'center', inline: 'nearest' }); }, [messagesEndRef]); diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index 926d9a8dac..ab58c30306 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -41,35 +41,37 @@ export const SnapshotTab: React.FunctionComponent<{ const [highlightedLocator, setHighlightedLocator] = React.useState(''); const [pickerVisible, setPickerVisible] = React.useState(false); - const snapshotMap = new Map(); - for (const snapshot of action?.snapshots || []) - snapshotMap.set(snapshot.title, snapshot); - const actionSnapshot = snapshotMap.get('action') || snapshotMap.get('after'); - const snapshots = [actionSnapshot ? { ...actionSnapshot, title: 'action' } : undefined, snapshotMap.get('before'), snapshotMap.get('after')].filter(Boolean) as { title: string, snapshotName: string }[]; - - let snapshotUrl = 'data:text/html,'; - let popoutUrl: string | undefined; - let snapshotInfoUrl: string | undefined; - let pointX: number | undefined; - let pointY: number | undefined; - if (action) { - const snapshot = snapshots[snapshotIndex]; - if (snapshot && snapshot.snapshotName) { - const params = new URLSearchParams(); - params.set('trace', context(action).traceUrl); - params.set('name', snapshot.snapshotName); - snapshotUrl = new URL(`snapshot/${action.pageId}?${params.toString()}`, window.location.href).toString(); - snapshotInfoUrl = new URL(`snapshotInfo/${action.pageId}?${params.toString()}`, window.location.href).toString(); - if (snapshot.snapshotName.includes('action')) { - pointX = action.point?.x; - pointY = action.point?.y; + const { snapshots, snapshotInfoUrl, snapshotUrl, pointX, pointY, popoutUrl } = React.useMemo(() => { + const snapshotMap = new Map(); + for (const snapshot of action?.snapshots || []) + snapshotMap.set(snapshot.title, snapshot); + const actionSnapshot = snapshotMap.get('action') || snapshotMap.get('after'); + const snapshots = [actionSnapshot ? { ...actionSnapshot, title: 'action' } : undefined, snapshotMap.get('before'), snapshotMap.get('after')].filter(Boolean) as { title: string, snapshotName: string }[]; + let snapshotUrl = 'data:text/html,'; + let popoutUrl: string | undefined; + let snapshotInfoUrl: string | undefined; + let pointX: number | undefined; + let pointY: number | undefined; + if (action) { + const snapshot = snapshots[snapshotIndex]; + if (snapshot && snapshot.snapshotName) { + const params = new URLSearchParams(); + params.set('trace', context(action).traceUrl); + params.set('name', snapshot.snapshotName); + snapshotUrl = new URL(`snapshot/${action.pageId}?${params.toString()}`, window.location.href).toString(); + snapshotInfoUrl = new URL(`snapshotInfo/${action.pageId}?${params.toString()}`, window.location.href).toString(); + if (snapshot.snapshotName.includes('action')) { + pointX = action.point?.x; + pointY = action.point?.y; + } + const popoutParams = new URLSearchParams(); + popoutParams.set('r', snapshotUrl); + popoutParams.set('trace', context(action).traceUrl); + popoutUrl = new URL(`popout.html?${popoutParams.toString()}`, window.location.href).toString(); } - const popoutParams = new URLSearchParams(); - popoutParams.set('r', snapshotUrl); - popoutParams.set('trace', context(action).traceUrl); - popoutUrl = new URL(`popout.html?${popoutParams.toString()}`, window.location.href).toString(); } - } + return { snapshots, snapshotInfoUrl, snapshotUrl, pointX, pointY, popoutUrl }; + }, [action, snapshotIndex]); React.useEffect(() => { if (snapshots.length >= 1 && snapshotIndex >= snapshots.length) @@ -155,8 +157,9 @@ export const SnapshotTab: React.FunctionComponent<{ setIsInspecting(!isInspecting); }}> { - setIsInspecting(false); + // Updating text needs to go first - react can squeeze a render between the state updates. setHighlightedLocator(text); + setIsInspecting(false); }}> { copy(highlightedLocator); diff --git a/packages/trace-viewer/src/ui/sourceTab.tsx b/packages/trace-viewer/src/ui/sourceTab.tsx index 47e07f5758..ff81a4fb62 100644 --- a/packages/trace-viewer/src/ui/sourceTab.tsx +++ b/packages/trace-viewer/src/ui/sourceTab.tsx @@ -70,7 +70,7 @@ export const SourceTab: React.FunctionComponent<{ const targetLine = typeof stackInfo === 'string' ? 0 : stackInfo.frames[selectedFrame]?.line || 0; - const targetLineRef = React.createRef(); + const targetLineRef = React.useRef(null); React.useLayoutEffect(() => { if (needReveal && targetLineRef.current) { targetLineRef.current.scrollIntoView({ block: 'center', inline: 'nearest' }); diff --git a/packages/web/src/components/codeMirrorWrapper.tsx b/packages/web/src/components/codeMirrorWrapper.tsx index 38b606634f..e44b83c0a3 100644 --- a/packages/web/src/components/codeMirrorWrapper.tsx +++ b/packages/web/src/components/codeMirrorWrapper.tsx @@ -42,15 +42,16 @@ export const CodeMirrorWrapper: React.FC = ({ text, language, readOnly, - highlight = [], + highlight, revealLine, lineNumbers, focusOnChange, wrapLines, onChange, }) => { - const codemirrorElement = React.createRef(); + const codemirrorElement = React.useRef(null); const [modulePromise] = React.useState>(import('./codeMirrorModule').then(m => m.default)); + const codemirrorRef = React.useRef(null); const [codemirror, setCodemirror] = React.useState(); React.useEffect(() => { @@ -70,18 +71,17 @@ export const CodeMirrorWrapper: React.FC = ({ if (language === 'csharp') mode = 'text/x-csharp'; - if (codemirror - && mode === codemirror.getOption('mode') - && readOnly === codemirror.getOption('readOnly') - && lineNumbers === codemirror.getOption('lineNumbers') - && wrapLines === codemirror.getOption('lineWrapping')) { - updateEditor(codemirror, text, highlight, revealLine, focusOnChange); + if (codemirrorRef.current + && mode === codemirrorRef.current.getOption('mode') + && readOnly === codemirrorRef.current.getOption('readOnly') + && lineNumbers === codemirrorRef.current.getOption('lineNumbers') + && wrapLines === codemirrorRef.current.getOption('lineWrapping')) { + // No need to re-create codemirror. return; } // Either configuration is different or we don't have a codemirror yet. - if (codemirror) - codemirror.getWrapperElement().remove(); + codemirrorRef.current?.getWrapperElement().remove(); const cm = CodeMirror(element, { value: '', @@ -90,29 +90,38 @@ export const CodeMirrorWrapper: React.FC = ({ lineNumbers, lineWrapping: wrapLines, }); + codemirrorRef.current = cm; setCodemirror(cm); - if (onChange) - cm.on('change', () => onChange(cm.getValue())); - updateEditor(cm, text, highlight, revealLine, focusOnChange); return cm; })(); - }, [modulePromise, codemirror, codemirrorElement, text, language, highlight, revealLine, focusOnChange, lineNumbers, wrapLines, readOnly, onChange]); + }, [modulePromise, codemirror, codemirrorElement, language, lineNumbers, wrapLines, readOnly]); + + React.useEffect(() => { + if (!codemirror) + return; + codemirror.off('change', (codemirror as any).listenerSymbol); + (codemirror as any)[listenerSymbol] = undefined; + if (onChange) { + (codemirror as any)[listenerSymbol] = () => onChange(codemirror.getValue()); + codemirror.on('change', (codemirror as any)[listenerSymbol]); + } + + if (codemirror.getValue() !== text) { + codemirror.setValue(text); + if (focusOnChange) { + codemirror.execCommand('selectAll'); + codemirror.focus(); + } + } + for (let i = 0; i < codemirror.lineCount(); ++i) + codemirror.removeLineClass(i, 'wrap'); + for (const h of highlight || []) + codemirror.addLineClass(h.line - 1, 'wrap', `source-line-${h.type}`); + if (revealLine) + codemirror.scrollIntoView({ line: revealLine - 1, ch: 0 }, 50); + }, [codemirror, text, highlight, revealLine, focusOnChange, onChange]); return
; }; -function updateEditor(cm: CodeMirror.Editor, text: string, highlight: SourceHighlight[], revealLine?: number, focusOnChange?: boolean) { - if (cm.getValue() !== text) { - cm.setValue(text); - if (focusOnChange) { - cm.execCommand('selectAll'); - cm.focus(); - } - } - for (let i = 0; i < cm.lineCount(); ++i) - cm.removeLineClass(i, 'wrap'); - for (const h of highlight) - cm.addLineClass(h.line - 1, 'wrap', `source-line-${h.type}`); - if (revealLine) - cm.scrollIntoView({ line: revealLine - 1, ch: 0 }, 50); -} +const listenerSymbol = Symbol('listener'); diff --git a/packages/web/src/components/listView.tsx b/packages/web/src/components/listView.tsx index 6865e30dc2..fcb1333f45 100644 --- a/packages/web/src/components/listView.tsx +++ b/packages/web/src/components/listView.tsx @@ -52,7 +52,7 @@ export function ListView({ noItemsMessage, dataTestId, }: ListViewProps) { - const itemListRef = React.createRef(); + const itemListRef = React.useRef(null); const [highlightedItem, setHighlightedItem] = React.useState(); React.useEffect(() => { diff --git a/packages/web/src/components/xtermWrapper.tsx b/packages/web/src/components/xtermWrapper.tsx index 1a1522199d..64c3fe015b 100644 --- a/packages/web/src/components/xtermWrapper.tsx +++ b/packages/web/src/components/xtermWrapper.tsx @@ -30,9 +30,9 @@ export type XtermDataSource = { export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ source }) => { - const xtermElement = React.createRef(); + const xtermElement = React.useRef(null); const [modulePromise] = React.useState>(import('./xtermModule').then(m => m.default)); - const [terminal, setTerminal] = React.useState(); + const terminal = React.useRef(null); React.useEffect(() => { (async () => { // Always load the module first. @@ -41,7 +41,7 @@ export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ if (!element) return; - if (terminal) + if (terminal.current) return; const newTerminal = new Terminal({ @@ -65,7 +65,7 @@ export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ }; newTerminal.open(element); fitAddon.fit(); - setTerminal(newTerminal); + terminal.current = newTerminal; const resizeObserver = new ResizeObserver(() => { source.resize(newTerminal.cols, newTerminal.rows); fitAddon.fit();