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.

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

ui: Read source UI improvements #3670

Merged
merged 8 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion ui/packages/shared/components/src/hooks/useURLState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ interface Props {
withURLUpdate?: boolean;
}

const isEqual = (a: string | string[] | undefined, b: string | string[] | undefined): boolean => {
if (typeof a === 'string' && typeof b === 'string') {
return a === b;
}
if (Array.isArray(a) && Array.isArray(b)) {
for (let i = 0; i < a.length; i++) {
if (a[i] !== b[i]) return false;
}
return true;
}
if (a === undefined && b === undefined) {
return true;
}
return false;
};

export const useURLState = ({
param,
navigateTo,
Expand Down Expand Up @@ -56,7 +72,7 @@ export const useURLState = ({
val === undefined || val == null || val === '';

if (withURLUpdate && navigateTo !== undefined) {
if (router[param] !== value) {
if (!isEqual(router[param], value)) {
const searchParams = router;
searchParams[param] = value;

Expand Down
22 changes: 14 additions & 8 deletions ui/packages/shared/profile/src/GraphTooltipArrow/Content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ const TooltipMetaInfo = ({
const locationLine: bigint = table.getChild(FIELD_LOCATION_LINE)?.get(row) ?? 0n;
const functionFilename: string = table.getChild(FIELD_FUNCTION_FILE_NAME)?.get(row) ?? '';
const functionStartLine: bigint = table.getChild(FIELD_FUNCTION_START_LINE)?.get(row) ?? 0n;
const lineNumber =
locationLine !== 0n ? locationLine : functionStartLine !== 0n ? functionStartLine : undefined;
const pprofLabelPrefix = 'pprof_labels.';
const labelColumnNames = table.schema.fields.filter(field =>
field.name.startsWith(pprofLabelPrefix)
Expand All @@ -209,23 +211,25 @@ const TooltipMetaInfo = ({
profileSource as ProfileSource,
QueryRequest_ReportType.SOURCE,
{
skip: enableSourcesView === false || profileSource === undefined,
skip:
enableSourcesView === false ||
profileSource === undefined ||
// eslint-disable-next-line no-extra-boolean-cast
!Boolean(mappingBuildID) ||
// eslint-disable-next-line no-extra-boolean-cast
!Boolean(functionFilename),
sourceBuildID: mappingBuildID,
sourceFilename: functionFilename,
sourceOnly: true,
}
);

const isSourceAvailable = !sourceLoading && sourceResponse != null;
const isSourceAvailable = !sourceLoading && sourceResponse?.report != null;

const getTextForFile = (): string => {
if (functionFilename === '') return '<unknown>';

return `${functionFilename} ${
locationLine !== 0n
? ` +${locationLine.toString()}`
: `${functionStartLine !== 0n ? `:${functionStartLine}` : ''}`
}`;
return `${functionFilename} ${lineNumber !== undefined ? ` +${lineNumber.toString()}` : ''}`;
};
const file = getTextForFile();

Expand Down Expand Up @@ -273,7 +277,9 @@ const TooltipMetaInfo = ({
setDashboardItems([dashboardItems[0], 'source']);
setSourceBuildId(mappingBuildID);
setSourceFilename(functionFilename);
setSourceLine(locationLine.toString());
if (lineNumber !== undefined) {
setSourceLine(lineNumber.toString());
}
};

return (
Expand Down
13 changes: 9 additions & 4 deletions ui/packages/shared/profile/src/SourceView/Highlighter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export const profileAwareRenderer = (
useInlineStyles,
}: RendererProps): JSX.Element {
const lineNumberWidth = charsToWidth(rows.length.toString().length);
const [sourceLine] = useURLState({param: 'source_line', navigateTo: () => {}});
const [sourceLine, setSourceLine] = useURLState({param: 'source_line', navigateTo: () => {}});
return (
<>
{rows.map((node, i) => {
Expand All @@ -129,11 +129,15 @@ export const profileAwareRenderer = (
<div className="flex gap-1" key={`${i}`}>
<div
className={cx(
'shrink-0 border-r border-gray-200 pr-1 text-right dark:border-gray-700',
'shrink-0 overflow-hidden border-r border-r-gray-200 text-right dark:border-r-gray-700',
lineNumberWidth
)}
>
<LineNo value={lineNumber} isCurrent={isCurrentLine} />
<LineNo
value={lineNumber}
isCurrent={isCurrentLine}
setCurrentLine={() => setSourceLine(lineNumber.toString())}
/>
</div>
<LineProfileMetadata
value={cumulative?.get(i) ?? 0n}
Expand Down Expand Up @@ -181,12 +185,13 @@ export const Highlighter = ({content, language, renderer}: HighlighterProps): JS
<div>Source</div>
</div>
</div>
<div className="h-[80vh] overflow-y-auto text-xs">
<div className="text-xs">
<SyntaxHighlighter
language={language}
style={isDarkMode ? atomOneDark : atomOneLight}
showLineNumbers
renderer={renderer}
customStyle={{padding: 0, height: '80vh'}}
>
{content}
</SyntaxHighlighter>
Expand Down
29 changes: 25 additions & 4 deletions ui/packages/shared/profile/src/SourceView/LineNo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,40 @@

import {useEffect, useRef} from 'react';

import cx from 'classnames';

interface Props {
value: number;
isCurrent?: boolean;
setCurrentLine?: () => void;
}

export const LineNo = ({value, isCurrent = false}: Props): JSX.Element => {
export const LineNo = ({value, isCurrent = false, setCurrentLine}: Props): JSX.Element => {
const ref = useRef<HTMLDivElement>(null);

useEffect(() => {
if (isCurrent) {
ref.current?.scrollIntoView({behavior: 'smooth', block: 'center'});
if (isCurrent && ref.current !== null) {
const bounds = ref.current.getBoundingClientRect();
if (
bounds.top > 0 &&
bounds.bottom < (window.innerHeight ?? document.documentElement.clientHeight)
) {
// already in view, so don't make the unnecessary scroll to center it
return;
}
ref.current.scrollIntoView({behavior: 'smooth', block: 'center'});
}
}, [isCurrent]);

return <code ref={ref}>{value.toString() + '\n'}</code>;
return (
<code
ref={ref}
onClick={setCurrentLine}
className={cx('cursor-pointer px-1', {
'border-l border-l-amber-900 bg-yellow-200 dark:bg-yellow-700': isCurrent,
})}
>
{value.toString() + '\n'}
</code>
);
};
12 changes: 10 additions & 2 deletions ui/packages/shared/profile/src/useQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,16 @@ export const useQuery = (
};
}

const {response} = await client.query(req, {meta: metadata});
return response;
try {
const {response} = await client.query(req, {meta: metadata});
return response;
} catch (e) {
if (options?.sourceOnly === true) {
// ignore
return {} as unknown as QueryResponse;
}
throw e;
}
},
options: {
retry: false,
Expand Down