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

perf: use redux state patches #26738

Merged
merged 28 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
abd5fab
Send state patches to UI
matthewwalsh0 Aug 29, 2024
c374367
Fix E2E tests
matthewwalsh0 Aug 29, 2024
28e3e41
Merge branch 'develop' into perf/redux-state-patches
matthewwalsh0 Aug 30, 2024
76f1608
Patch and apply with custom functions
matthewwalsh0 Sep 3, 2024
315e20f
Add E2E logging
matthewwalsh0 Sep 3, 2024
0ff9ac0
Filter state patches
matthewwalsh0 Sep 4, 2024
0744efd
Add logging
matthewwalsh0 Sep 4, 2024
d724508
Enable lavamoat
matthewwalsh0 Sep 4, 2024
4bf695d
Skip import time logging
matthewwalsh0 Sep 4, 2024
11caf25
Extend logging
matthewwalsh0 Sep 4, 2024
7a9f16b
Merge branch 'develop' into perf/redux-state-patches
matthewwalsh0 Sep 4, 2024
1e27752
Delay state sync
matthewwalsh0 Sep 4, 2024
965ec51
Remove full state updates
matthewwalsh0 Sep 4, 2024
ea7fc87
Fix storybook
matthewwalsh0 Sep 4, 2024
941e259
Revert unnecessary changes
matthewwalsh0 Sep 6, 2024
3577c6a
Merge branch 'develop' into perf/redux-state-patches
matthewwalsh0 Sep 10, 2024
3c4e8ae
Fix linting
matthewwalsh0 Sep 10, 2024
c23ad1d
Add PatchStore unit tests
matthewwalsh0 Sep 10, 2024
485f6b3
Fix unit tests
matthewwalsh0 Sep 11, 2024
f07d097
Minimise patch array
matthewwalsh0 Sep 11, 2024
c672c51
Create state utils
matthewwalsh0 Sep 11, 2024
1438d3b
Fix linting
matthewwalsh0 Sep 11, 2024
b5f22e8
Fix E2E tests
matthewwalsh0 Sep 11, 2024
b5968fe
Merge branch 'develop' into perf/redux-state-patches
matthewwalsh0 Sep 14, 2024
54bbdf5
Merge branch 'develop' into perf/redux-state-patches
matthewwalsh0 Sep 18, 2024
5f17e8d
Remove duplicate state update
matthewwalsh0 Sep 19, 2024
39cb672
Merge branch 'develop' into perf/redux-state-patches
matthewwalsh0 Sep 19, 2024
86ab71d
Fix unit test
matthewwalsh0 Sep 19, 2024
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
2 changes: 0 additions & 2 deletions app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,6 @@ export const SENTRY_BACKGROUND_STATE = {
},
},
SnapController: {
unencryptedSnapStates: false,
snapStates: false,
snaps: false,
},
SnapInterfaceController: {
Expand Down
24 changes: 16 additions & 8 deletions app/scripts/controllers/preferences-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,11 @@ export default class PreferencesController {
*/
addKnownMethodData(fourBytePrefix: string, methodData: string): void {
const { knownMethodData } = this.store.getState();
knownMethodData[fourBytePrefix] = methodData;
this.store.updateState({ knownMethodData });

const updatedKnownMethodData = { ...knownMethodData };
updatedKnownMethodData[fourBytePrefix] = methodData;

this.store.updateState({ knownMethodData: updatedKnownMethodData });
}

/**
Expand Down Expand Up @@ -765,11 +768,16 @@ export default class PreferencesController {
const addresses = Object.values(accounts).map((account) =>
account.address.toLowerCase(),
);
Object.keys(identities).forEach((identity) => {
if (addresses.includes(identity.toLowerCase())) {
lostIdentities[identity] = identities[identity];
}
});

const updatedLostIdentities = Object.keys(identities).reduce(
(acc, identity) => {
if (addresses.includes(identity.toLowerCase())) {
acc[identity] = identities[identity];
}
return acc;
},
{ ...(lostIdentities ?? {}) },
);

const updatedIdentities = Object.values(accounts).reduce(
(identitiesMap: Record<string, AccountIdentityEntry>, account) => {
Expand All @@ -786,7 +794,7 @@ export default class PreferencesController {

this.store.updateState({
identities: updatedIdentities,
lostIdentities,
lostIdentities: updatedLostIdentities,
selectedAddress: selectedAccount?.address || '', // it will be an empty string during onboarding
});
}
Expand Down
12 changes: 10 additions & 2 deletions app/scripts/lib/ComposableObservableStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default class ComposableObservableStore extends ObservableStore {
const store = config[key];
if (store.subscribe) {
config[key].subscribe((state) => {
this.updateState({ [key]: state });
this.#onStateChange(key, state);
});
} else {
this.controllerMessenger.subscribe(
Expand All @@ -69,7 +69,7 @@ export default class ComposableObservableStore extends ObservableStore {
if (this.persist) {
updatedState = getPersistentState(state, config[key].metadata);
}
this.updateState({ [key]: updatedState });
this.#onStateChange(key, updatedState);
},
);
}
Expand Down Expand Up @@ -104,4 +104,12 @@ export default class ComposableObservableStore extends ObservableStore {
}
return flatState;
}

#onStateChange(controllerKey, newState) {
const oldState = this.getState()[controllerKey];

this.updateState({ [controllerKey]: newState });

this.emit('stateChange', { oldState, newState, controllerKey });
}
}
183 changes: 183 additions & 0 deletions app/scripts/lib/PatchStore.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
import ComposableObservableStore from './ComposableObservableStore';
import { PatchStore } from './PatchStore';
import { sanitizeUIState } from './state-utils';

jest.mock('./state-utils');

function createComposableStoreMock() {
return {
on: jest.fn(),
removeListener: jest.fn(),
} as unknown as jest.Mocked<ComposableObservableStore>;
}

function triggerStateChange(
composableStoreMock: jest.Mocked<ComposableObservableStore>,
oldState: Record<string, unknown>,
newState: Record<string, unknown>,
) {
composableStoreMock.on.mock.calls[0][1]({
controllerKey: 'test-controller',
oldState,
newState,
});
}

describe('PatchStore', () => {
const sanitizeUIStateMock = jest.mocked(sanitizeUIState);

beforeEach(() => {
jest.resetAllMocks();
sanitizeUIStateMock.mockImplementation((state) => state);
});

describe('flushPendingPatches', () => {
it('returns pending patches created by composable store events', () => {
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

triggerStateChange(
composableStoreMock,
{ test1: 'value1' },
{ test1: 'value2' },
);

triggerStateChange(
composableStoreMock,
{ test2: true },
{ test2: false },
);

const patches = patchStore.flushPendingPatches();

expect(patches).toEqual([
{
op: 'replace',
path: ['test1'],
value: 'value2',
},
{
op: 'replace',
path: ['test2'],
value: false,
},
]);
});

it('ignores state properties if old and new state is shallow equal', () => {
const objectMock = {};
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

triggerStateChange(
composableStoreMock,
{ test1: 'value1' },
{ test1: 'value1' },
);

triggerStateChange(
composableStoreMock,
{ test2: objectMock },
{ test2: objectMock },
);

triggerStateChange(
composableStoreMock,
{ test3: { test: 'value' } },
{ test3: { test: 'value' } },
);

const patches = patchStore.flushPendingPatches();

expect(patches).toEqual([
{
op: 'replace',
path: ['test3'],
value: { test: 'value' },
},
]);
});

it('returns empty array if no composable store events', () => {
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

const patches = patchStore.flushPendingPatches();

expect(patches).toEqual([]);
});

it('clears pending patches', () => {
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

triggerStateChange(
composableStoreMock,
{ test1: 'value1' },
{ test1: 'value2' },
);

const patches1 = patchStore.flushPendingPatches();
const patches2 = patchStore.flushPendingPatches();

expect(patches1).toHaveLength(1);
expect(patches2).toHaveLength(0);
});

it('sanitizes state in patches', () => {
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

sanitizeUIStateMock.mockReturnValueOnce({ test2: 'value' });

triggerStateChange(
composableStoreMock,
{ test1: false },
{ test1: true },
);

const patches = patchStore.flushPendingPatches();

expect(patches).toEqual([
{
op: 'replace',
path: ['test2'],
value: 'value',
},
]);
});

it('adds isInitialized patch if vault in new state', () => {
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

triggerStateChange(composableStoreMock, { vault: 0 }, { vault: 123 });

const patches = patchStore.flushPendingPatches();

expect(patches).toEqual([
{
op: 'replace',
path: ['vault'],
value: 123,
},
{
op: 'replace',
path: ['isInitialized'],
value: true,
},
]);
});
});

describe('destroy', () => {
it('removes listener from composable store', () => {
const composableStoreMock = createComposableStoreMock();
const patchStore = new PatchStore(composableStoreMock);

patchStore.destroy();

expect(composableStoreMock.removeListener).toHaveBeenCalledTimes(1);
});
});
});
103 changes: 103 additions & 0 deletions app/scripts/lib/PatchStore.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { createProjectLogger } from '@metamask/utils';
import { Patch } from 'immer';
import { v4 as uuid } from 'uuid';
import ComposableObservableStore from './ComposableObservableStore';
import { sanitizeUIState } from './state-utils';

const log = createProjectLogger('patch-store');

export class PatchStore {
private id: string;

private observableStore: ComposableObservableStore;

private pendingPatches: Map<string, Patch> = new Map();

private listener: (request: {
controllerKey: string;
oldState: Record<string, unknown>;
newState: Record<string, unknown>;
}) => void;

constructor(observableStore: ComposableObservableStore) {
this.id = uuid();
this.observableStore = observableStore;
this.listener = this._onStateChange.bind(this);

this.observableStore.on('stateChange', this.listener);

log('Created', this.id);
}

flushPendingPatches(): Patch[] {
const patches = [...this.pendingPatches.values()];

this.pendingPatches.clear();

for (const patch of patches) {
log('Flushed', patch.path.join('.'), this.id, patch);
}

return patches;
}

destroy() {
this.observableStore.removeListener('stateChange', this.listener);
log('Destroyed', this.id);
}

private _onStateChange({
oldState,
newState,
}: {
controllerKey: string;
oldState: Record<string, unknown>;
newState: Record<string, unknown>;
}) {
const sanitizedNewState = sanitizeUIState(newState);
const patches = this._generatePatches(oldState, sanitizedNewState);
const isInitialized = Boolean(newState.vault);

if (isInitialized) {
patches.push({
op: 'replace',
path: ['isInitialized'],
value: isInitialized,
});
}

if (!patches.length) {
return;
}

for (const patch of patches) {
const path = patch.path.join('.');

this.pendingPatches.set(path, patch);

log('Updated', path, this.id, patch);
}
}

private _generatePatches(
oldState: Record<string, unknown>,
newState: Record<string, unknown>,
): Patch[] {
return Object.keys(newState)
.map((key) => {
const oldData = oldState[key];
const newData = newState[key];

if (oldData === newData) {
return null;
}

return {
op: 'replace',
path: [key],
value: newData,
};
})
.filter(Boolean) as Patch[];
}
}
Loading