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

Refactor react perf profiler for local dev, local data collection #6186

Merged
merged 1 commit into from
Jun 16, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/healthy-maps-watch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Add react profiler log data option to storybook globals
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, {PropsWithChildren} from 'react';
import isChromatic from 'chromatic/isChromatic';
import {version} from '../../package.json';

interface Data {
Expand All @@ -15,35 +14,26 @@ interface ProfileProps {
kind: string;
}

const isDevelopment = process.env.NODE_ENV === 'development';

const trackRenderPerformance = (data: Data) => {
const commitSha = process.env.STORYBOOK_GITHUB_SHA
? process.env.STORYBOOK_GITHUB_SHA
: 'localdev';

const body = JSON.stringify({...data, commitSha, version});

const target = isDevelopment
? '//localhost:3000/api/profiler'
: 'https://polaris-coverage.shopifycloud.com/api/profiler';

fetch(target, {
method: 'POST',
keepalive: true,
mode: 'no-cors',
body,
});
console.log({...data, commitSha, version});
};

const Profiler = ({id, kind, children}: PropsWithChildren<ProfileProps>) => (
export const RenderPerformanceProfiler = ({
id,
kind,
children,
}: PropsWithChildren<ProfileProps>) => (
<React.Profiler
id={id}
// https://reactjs.org/docs/profiler.html#onrender-callback
onRender={(_, phase, actualDuration, baseDuration) => {
trackRenderPerformance({
id,
kind,
kind: kind.replace('All Components/', ''),
phase,
actualDuration,
baseDuration,
Expand All @@ -53,8 +43,3 @@ const Profiler = ({id, kind, children}: PropsWithChildren<ProfileProps>) => (
{children}
</React.Profiler>
);

const Component = ({children}: PropsWithChildren<{}>) => <>{children}</>;

export const RenderPerformanceProfiler =
isDevelopment || isChromatic() ? Profiler : Component;
4 changes: 2 additions & 2 deletions polaris-react/.storybook/manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ function GridPanel(props) {
name: 'Show grid overlay',
description:
'Show or hide a 4 / 12 column grid, overlaying components',
defaultValue: false,
defaultValue: 'false',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work with booleans?! 😰

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with both, just making this change for consistency

control: {type: 'boolean'},
},
gridInFrame: {
name: 'Grid in frame',
description: 'Show grid within app frame context',
defaultValue: false,
defaultValue: 'false',
control: {type: 'boolean'},
},
gridWidth: {
Expand Down
25 changes: 20 additions & 5 deletions polaris-react/.storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,15 @@ function GridOverlayDecorator(Story, context) {
);
}

function RenderPerformanceProfilerDecorator(Story, context) {
function ReactRenderProfiler(Story, context) {
const {profiler} = context.globals;
const Wrapper = profiler ? RenderPerformanceProfiler : React.Fragment;
const props = profiler ? {id: context.id, kind: context.kind} : {};

return (
<RenderPerformanceProfiler id={context.id} kind={context.kind}>
<Wrapper {...props}>
<Story {...context} />
</RenderPerformanceProfiler>
</Wrapper>
);
}

Expand All @@ -58,7 +62,18 @@ export const globalTypes = {
{title: 'Disabled', value: 'false'},
{title: 'Enabled', value: 'true'},
],
showName: true,
showName: 'true',
},
},
profiler: {
name: 'React.Profiler',
defaultValue: 'false',
toolbar: {
items: [
{title: 'Disabled', value: 'false'},
{title: 'Enabled', value: 'true'},
],
showName: 'true',
},
},
};
Expand All @@ -67,5 +82,5 @@ export const decorators = [
GridOverlayDecorator,
StrictModeDecorator,
AppProviderDecorator,
RenderPerformanceProfilerDecorator,
ReactRenderProfiler,
];