Skip to content

Commit

Permalink
fix: Improve stability of initial state sync in Snaps UI (#25375)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Improves stability of initial state sync when using Snaps UI.
Previously, sometimes, the `initialState` in the `SnapInterfaceContext`
wouldn't be updated when the content of the interface changed and thus
initial values would be missing.

This PR changes the implementation to use one selector in the renderer,
which only changes when the content does, but passes the `initialState`
and `context` to the `SnapInterfaceContext` itself. This way we are
guaranteed for the content and state to stay in sync.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25375?quickstart=1)


Can be tested with a Snap like:

```ts
export const onRpcRequest: OnRpcRequestHandler = async ({
  origin,
  request,
}) => {
  switch (request.method) {
    case 'hello': {
      const id = await snap.request({
        method: 'snap_createInterface',
        params: {
          ui: (
            <Box>
              <Text>foo</Text>
              <Button>Click me</Button>
            </Box>
          ),
          context: { foo: 'bar' },
        },
      });

      return snap.request({
        method: 'snap_dialog',
        params: {
          type: 'confirmation',
          id,
        },
      });
    }
    default:
      throw new Error('Method not found.');
  }
};

export const onUserInput: OnUserInputHandler = async ({
  id,
  event,
  context,
}) => {
  if (event.type === 'ButtonClickEvent') {
    await snap.request({
      method: 'snap_updateInterface',
      params: {
        id,
        ui: (
          <Box>
            <Heading>Settings</Heading>
            <Input name="foo" value="foo" />
            <Form name="settingsForm">
              <Field label="foo">
                <Input name="foo" value="foo" />
              </Field>
            </Form>
          </Box>
        ),
      },
    });
    return;
  }
};

```
  • Loading branch information
FrederikBolding authored Jun 18, 2024
1 parent f6890a7 commit 45c9466
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
23 changes: 16 additions & 7 deletions ui/components/app/snaps/snap-ui-renderer/snap-ui-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import { useSelector } from 'react-redux';
import { isEqual } from 'lodash';
import MetaMaskTemplateRenderer from '../../metamask-template-renderer/metamask-template-renderer';
import { SnapDelineator } from '../snap-delineator';
import {
getSnapMetadata,
getMemoizedInterfaceContent,
} from '../../../../selectors';
import { getSnapMetadata, getMemoizedInterface } from '../../../../selectors';
import { Box, FormTextField } from '../../../component-library';
import { DelineatorType } from '../../../../helpers/constants/snaps';

Expand All @@ -35,10 +32,15 @@ const SnapUIRendererComponent = ({
getSnapMetadata(state, snapId),
);

const content = useSelector((state) =>
getMemoizedInterfaceContent(state, interfaceId),
const interfaceState = useSelector(
(state) => getMemoizedInterface(state, interfaceId),
// We only want to update the state if the content has changed.
// We do this to avoid useless re-renders.
(oldState, newState) => isEqual(oldState.content, newState.content),
);

const content = interfaceState?.content;

// sections are memoized to avoid useless re-renders if one of the parents element re-renders.
const sections = useMemo(
() =>
Expand All @@ -64,6 +66,8 @@ const SnapUIRendererComponent = ({
);
}

const { state: initialState, context } = interfaceState;

return (
<SnapDelineator
snapName={snapName}
Expand All @@ -74,7 +78,12 @@ const SnapUIRendererComponent = ({
boxProps={boxProps}
>
<Box className="snap-ui-renderer__content">
<SnapInterfaceContextProvider snapId={snapId} interfaceId={interfaceId}>
<SnapInterfaceContextProvider
snapId={snapId}
interfaceId={interfaceId}
initialState={initialState}
context={context}
>
<MetaMaskTemplateRenderer sections={sections} />
</SnapInterfaceContextProvider>
{isPrompt && (
Expand Down
16 changes: 7 additions & 9 deletions ui/contexts/snaps/snap-interface.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
InterfaceState,
UserInputEventType,
} from '@metamask/snaps-sdk';
import { Json } from '@metamask/utils';
import { debounce, throttle } from 'lodash';
import React, {
FunctionComponent,
Expand All @@ -11,8 +12,7 @@ import React, {
useEffect,
useRef,
} from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { getMemoizedInterface } from '../../selectors';
import { useDispatch } from 'react-redux';
import {
handleSnapRequest,
updateInterfaceState,
Expand Down Expand Up @@ -47,6 +47,8 @@ export const SnapInterfaceContext =
export type SnapInterfaceContextProviderProps = {
interfaceId: string;
snapId: string;
initialState: Record<string, string | Record<string, unknown> | unknown>;
context: Json;
};

// We want button clicks to be instant and therefore use throttling
Expand All @@ -64,18 +66,14 @@ const THROTTLED_EVENTS = [
* @param params.children - The childrens to wrap with the context provider.
* @param params.interfaceId - The interface ID to use.
* @param params.snapId - The Snap ID that requested the interface.
* @param params.initialState - The initial state of the interface.
* @param params.context - The context blob of the interface.
* @returns The context provider.
*/
export const SnapInterfaceContextProvider: FunctionComponent<
SnapInterfaceContextProviderProps
> = ({ children, interfaceId, snapId }) => {
> = ({ children, interfaceId, snapId, initialState, context }) => {
const dispatch = useDispatch();
const { state: initialState, context } = useSelector(
(state) => getMemoizedInterface(state, interfaceId),
// Prevents the selector update.
// We do this to avoid useless re-renders.
() => true,
);

// We keep an internal copy of the state to speed-up the state update in the UI.
// It's kept in a ref to avoid useless re-rendering of the entire tree of components.
Expand Down

0 comments on commit 45c9466

Please sign in to comment.