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

Avoid too many binding evaluations in the runtime #2142

Merged
merged 22 commits into from
Jun 12, 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
1,771 changes: 971 additions & 800 deletions packages/toolpad-app/src/runtime/ToolpadApp.tsx

Large diffs are not rendered by default.

32 changes: 0 additions & 32 deletions packages/toolpad-app/src/runtime/toolpadComponents/template.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,23 +1,30 @@
import * as React from 'react';
import {
ApplicationVm,
ArgTypeDefinition,
BindableAttrValue,
DEFAULT_LOCAL_SCOPE_PARAMS,
LocalScopeParams,
RuntimeScope,
ScopeMeta,
ScopeMetaField,
} from '@mui/toolpad-core';
import { Alert, Box } from '@mui/material';
import { useBrowserJsRuntime } from '@mui/toolpad-core/jsBrowserRuntime';
import { mapValues } from '@mui/toolpad-utils/collections';
import * as appDom from '../../../appDom';
import { useDom, useDomApi } from '../../AppState';
import { useDomApi } from '../../AppState';
import BindableEditor from './BindableEditor';
import { usePageEditorState } from './PageEditorProvider';
import { getDefaultControl } from '../../propertyControls';
import { isTemplateDescendant } from '../../../runtime/toolpadComponents/template';
import { NON_BINDABLE_CONTROL_TYPES } from '../../../runtime/constants';

function buildScopeMeta(vm: ApplicationVm, bindingScope?: RuntimeScope): ScopeMeta {
if (bindingScope?.parentScope) {
return {
...buildScopeMeta(vm, bindingScope?.parentScope),
...bindingScope?.meta,
};
}
return bindingScope?.meta ?? {};
}

export interface NodeAttributeEditorProps<P extends object, K extends keyof P = keyof P> {
node: appDom.AppDomNode;
namespace?: string;
Expand All @@ -33,7 +40,6 @@ export default function NodeAttributeEditor<P extends object>({
argType,
props,
}: NodeAttributeEditorProps<P>) {
const { dom } = useDom();
const domApi = useDomApi();

const handlePropChange = React.useCallback(
Expand All @@ -48,9 +54,14 @@ export default function NodeAttributeEditor<P extends object>({
const propValue: BindableAttrValue<unknown> | null = (node as any)[namespace]?.[name] ?? null;

const bindingId = `${node.id}${namespace ? `.${namespace}` : ''}.${name}`;
const { bindings, pageState, globalScopeMeta, viewState } = usePageEditorState();
const { vm } = usePageEditorState();

const scopeId = vm.bindingScopes[bindingId];
const bindingScope = scopeId ? vm.scopes[scopeId] : undefined;

const liveBinding = bindingScope?.bindings[bindingId];

const liveBinding = bindings[bindingId];
const scopeMeta = React.useMemo(() => buildScopeMeta(vm, bindingScope), [vm, bindingScope]);

const Control = getDefaultControl(argType, props);

Expand All @@ -65,27 +76,11 @@ export default function NodeAttributeEditor<P extends object>({

const jsBrowserRuntime = useBrowserJsRuntime();

const isNodeTemplateDescendant = React.useMemo(
() => appDom.isElement(node) && isTemplateDescendant(dom, node, viewState),
[dom, node, viewState],
);

const localState: LocalScopeParams = isNodeTemplateDescendant
? { i: DEFAULT_LOCAL_SCOPE_PARAMS.i }
: {};
const localScopeMeta: ScopeMeta = mapValues(
localState,
() => ({ kind: 'local' } as ScopeMetaField),
);

return Control ? (
<BindableEditor
liveBinding={liveBinding}
globalScope={{ ...pageState, ...localState }}
globalScopeMeta={{
...globalScopeMeta,
...localScopeMeta,
}}
globalScope={bindingScope?.values ?? {}}
globalScopeMeta={scopeMeta}
label={argType.label || name}
bindable={isBindable}
disabled={isDisabled}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { NodeId, LiveBindings, ScopeMeta } from '@mui/toolpad-core';
import { NodeId, LiveBindings, ScopeMeta, ApplicationVm } from '@mui/toolpad-core';
import * as React from 'react';
import * as appDom from '../../../appDom';
import { PageViewState } from '../../../types';
Expand Down Expand Up @@ -31,6 +31,7 @@ export interface PageEditorState {
readonly pageState: Record<string, unknown>;
readonly globalScopeMeta: ScopeMeta;
readonly bindings: LiveBindings;
readonly vm: ApplicationVm;
}

export type PageEditorAction =
Expand Down Expand Up @@ -76,6 +77,10 @@ export type PageEditorAction =
| {
type: 'PAGE_BINDINGS_UPDATE';
bindings: LiveBindings;
}
| {
type: 'VM_UPDATE';
vm: ApplicationVm;
};

export function createPageEditorState(nodeId: NodeId): PageEditorState {
Expand All @@ -93,6 +98,10 @@ export function createPageEditorState(nodeId: NodeId): PageEditorState {
pageState: {},
globalScopeMeta: {},
bindings: {},
vm: {
scopes: {},
bindingScopes: {},
},
};
}

Expand Down Expand Up @@ -164,6 +173,10 @@ export function pageEditorReducer(
bindings,
});
}
case 'VM_UPDATE': {
const { vm } = action;
return update(state, { vm });
}
default:
return state;
}
Expand Down Expand Up @@ -220,6 +233,12 @@ function createPageEditorApi(dispatch: React.Dispatch<PageEditorAction>) {
bindings,
});
},
vmUpdate(vm: ApplicationVm) {
dispatch({
type: 'VM_UPDATE',
vm,
});
},
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ export default function RenderPanel({ className }: RenderPanelProps) {
pageEditorApi.pageBindingsUpdate(event.bindings);
});

initializedBridge.canvasEvents.on('vmUpdated', (event) => {
pageEditorApi.vmUpdate(event.vm);
});

initializedBridge.canvasEvents.on('screenUpdate', () => {
const pageViewState = initializedBridge.canvasCommands.getPageViewState();
pageEditorApi.pageViewStateUpdate(pageViewState);
Expand Down
2 changes: 1 addition & 1 deletion packages/toolpad-components/src/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function List({ itemCount, renderItem, disablePadding = false, sx }: ListProps)
<MuiList disablePadding={disablePadding} sx={{ width: '100%', ...sx }}>
{[...Array(itemCount).keys()].map((index) => (
<ListItem key={index} disablePadding={disablePadding}>
<Box sx={{ width: '100%', p: 0, m: 0 }}>{renderItem({ i: index })}</Box>
<Box sx={{ width: '100%', p: 0, m: 0 }}>{renderItem(`item-${index}`, { i: index })}</Box>
</ListItem>
))}
</MuiList>
Expand Down
2 changes: 0 additions & 2 deletions packages/toolpad-core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,3 @@ export const TOOLPAD_FUNCTION = Symbol.for('ToolpadFunction');
export const TOOLPAD_COMPONENT_MODE_PROPERTY = 'ToolpadComponentMode';

export const IMAGE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'gif', 'svg', 'webp'];

export const DEFAULT_LOCAL_SCOPE_PARAMS = { i: 0 };
21 changes: 16 additions & 5 deletions packages/toolpad-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ export type RuntimeEvents = {
screenUpdate: {};
ready: {};
pageNavigationRequest: { pageNodeId: NodeId };
vmUpdated: { vm: ApplicationVm };
};

export type RuntimeEvent = {
Expand Down Expand Up @@ -477,10 +478,20 @@ export interface JsRuntime {
evaluateExpression(code: string, globalScope: Record<string, unknown>): BindingEvaluationResult;
}

export type LocalScopeParams = Record<string, unknown>;

export interface TemplateScopeParams {
i: number;
export type TemplateRenderer = (
scopeKey: string,
params: Record<string, unknown>,
) => React.ReactNode;

export interface RuntimeScope {
id: string;
parentScope?: RuntimeScope;
bindings: Record<string, BindingEvaluationResult<unknown>>;
values: Record<string, unknown>;
meta: ScopeMeta;
}

export type TemplateRenderer = ({ i }: TemplateScopeParams) => React.ReactNode;
export interface ApplicationVm {
scopes: { [id in string]?: RuntimeScope };
bindingScopes: { [id in string]?: string };
}
2 changes: 1 addition & 1 deletion packages/toolpad-utils/src/collections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function hasOwnProperty<X extends {}, Y extends PropertyKey>(
}

/**
* Maps `obj` to a new object. The `mapper` function receives an entry array of jey and value and
* Maps `obj` to a new object. The `mapper` function receives an entry array of key and value and
* is allowed to manipulate both. It can also return `null` to omit a property from the result.
*/
export function mapProperties<P, L extends PropertyKey, U>(
Expand Down
31 changes: 31 additions & 0 deletions packages/toolpad-utils/src/react.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,34 @@ export function createProvidedContext<T>(
const useContext = () => useNonNullableContext(context, name);
return [useContext, context.Provider as React.ComponentType<React.ProviderProps<T>>];
}

export function useAssertedContext<T>(context: React.Context<T | undefined>): T {
const value = React.useContext(context);
if (value === undefined) {
throw new Error('context was used without a Provider');
}
return value;
}

/**
* Debugging tool that logs updates to props.
*/
export function useTraceUpdates<P extends object>(prefix: string, props: P) {
const prev = React.useRef<P>(props);
React.useEffect(() => {
const changedProps: Partial<P> = {};

for (const key of Object.keys(props) as (keyof P)[]) {
if (!Object.is(prev.current[key], props[key])) {
changedProps[key] = props[key];
}
}

if (Object.keys(changedProps).length > 0) {
// eslint-disable-next-line no-console
console.log(`${prefix} changed props:`, changedProps);
}

prev.current = props;
});
}
Loading