Skip to content

Commit 03a6c5b

Browse files
committed
chore(keyring-sdk): fix review issues in migration module
- Remove broken {@link defineMigrations} reference from JSDoc - Add test: version-0 versioned state triggers all migrations - Add test: default JsonStruct validation fires without inputSchema - Fix serialize() example to use VersionedState<HdStateV2> return type - Add `as const` guidance to Best Practices in migrations.md
1 parent 58defb8 commit 03a6c5b

3 files changed

Lines changed: 49 additions & 4 deletions

File tree

packages/keyring-sdk/docs/migrations.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ const migrations = [
8181

8282
```typescript
8383
import { applyMigrations, getLatestVersion } from '@metamask/keyring-sdk';
84+
import type { VersionedState } from '@metamask/keyring-sdk';
8485
import type { Json } from '@metamask/utils';
8586

8687
class MyKeyring {
@@ -93,21 +94,22 @@ class MyKeyring {
9394
this.#hdPath = data.hdPath;
9495
}
9596

96-
async serialize(): Promise<Json> {
97+
async serialize(): Promise<VersionedState<HdStateV2>> {
9798
return {
9899
version: getLatestVersion(migrations),
99100
data: {
100101
mnemonic: this.#mnemonic,
101102
accountCount: this.#accountCount,
102103
hdPath: this.#hdPath,
103104
},
104-
} as Json;
105+
};
105106
}
106107
}
107108
```
108109

109110
## Best Practices
110111

112+
- **`as const` arrays**: Declare the migrations array with `as const` so TypeScript infers the final state type from `applyMigrations`. Without it, `data` falls back to `Json`.
111113
- **Idempotent migrations**: Design migrations so re-running them on already-migrated data is harmless.
112114
- **Immutability**: Treat the input `data` as immutable within the `migrate` function.
113115
- **Schema coverage**: Ensure `schema` covers all fields expected in the new version to prevent runtime errors.

packages/keyring-sdk/src/migration.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,22 @@ describe('defineMigration', () => {
157157

158158
expect(migrateFn).not.toHaveBeenCalled();
159159
});
160+
161+
it('validates input as JSON even when inputSchema is omitted', async () => {
162+
const migrateFn = jest.fn();
163+
const migrations = [
164+
defineMigration({ version: 1, migrate: migrateFn }),
165+
] as const;
166+
167+
// undefined is not valid JSON
168+
await expect(
169+
applyMigrations(undefined as unknown as Json, migrations),
170+
).rejects.toThrow(
171+
'Expected a value of type `JSON`, but received: `undefined`',
172+
);
173+
174+
expect(migrateFn).not.toHaveBeenCalled();
175+
});
160176
});
161177
});
162178

@@ -336,6 +352,33 @@ describe('applyMigrations', () => {
336352
migrated: true,
337353
});
338354
});
355+
356+
it('treats explicitly versioned state at version 0 as unversioned', async () => {
357+
const migrations = [
358+
defineMigration<HdStateV1>({
359+
version: 1,
360+
migrate: (state) => {
361+
const prev = state as UnversionedHdState;
362+
return { ...prev, newField: 'added' };
363+
},
364+
}),
365+
] as const;
366+
367+
const result = await applyMigrations(
368+
{ version: 0, data: { oldField: 'value', existing: true } },
369+
migrations,
370+
);
371+
372+
expect(result).toStrictEqual({
373+
version: 1,
374+
data: {
375+
oldField: 'value',
376+
existing: true,
377+
newField: 'added',
378+
} satisfies HdStateV1,
379+
migrated: true,
380+
});
381+
});
339382
});
340383

341384
describe('when migrate is async', () => {

packages/keyring-sdk/src/migration.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ export type KeyringMigration<
6969
* called, and the `Input` type parameter is inferred so `migrate` receives a typed
7070
* argument with no manual cast needed.
7171
*
72-
* Returns `KeyringMigration<Output>` so that {@link defineMigrations} and
73-
* {@link applyMigrations} can infer the output type.
72+
* Returns `KeyringMigration<Output>` so that {@link applyMigrations} can infer the
73+
* output type.
7474
*
7575
* @param config - The migration configuration.
7676
* @param config.version - The version this migration produces.

0 commit comments

Comments
 (0)