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: split keyring-api #24

Merged
merged 56 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
33a9c94
feat(keyring-utils): add new package @metamask/keyring-utils
ccharly Sep 6, 2024
fec963e
feat(keyring-internal-api): add new package @metamask/keyring-interna…
ccharly Sep 9, 2024
9ee2922
feat(keyring-snap-sdk): add new package @metamask/keyring-snap-sdk
ccharly Sep 9, 2024
58d80eb
feat(keyring-snap-client): add new package @metamask/keyring-snap-client
ccharly Sep 9, 2024
97b67b4
fix(keyring-internal-api): silent some eslint rules
ccharly Sep 10, 2024
6e515a3
fix(keyring-snap-sdk): silent some eslint rules
ccharly Sep 10, 2024
3dbb7de
fix(keyring-snap-client): silent some eslint rules
ccharly Sep 10, 2024
ca8561d
fix(keyring-internal-api): silent eslint type-constitents rules
ccharly Nov 26, 2024
13214bf
build: use local @metamask/keyring-{utils,interntal-api,snap-sdk,snap…
ccharly Sep 9, 2024
2265d9e
chore: update README.md
ccharly Sep 6, 2024
908ea84
chore(syncpack): use workspace:^ for new packages
ccharly Nov 26, 2024
8d4d998
build: build new packages with ts-bridge
ccharly Nov 26, 2024
247c3e7
test(keyring-utils): fix coverage
ccharly Nov 26, 2024
955a0d6
test(keyring-api): fix coverage
ccharly Nov 27, 2024
bc68ebc
chore: f fix coverage keyring-utils
ccharly Nov 27, 2024
eb9204b
test(keyring-internal-api): fix coverage
ccharly Nov 27, 2024
a233021
refactor(keyring-api): flatten folders
ccharly Nov 27, 2024
03b206c
refactor(keyring-internal-api): flatten folders
ccharly Nov 27, 2024
8be617b
test(keyring-internal-api): add tests for Keyring RPC methods
ccharly Nov 27, 2024
b89588d
fix: fix types field in package.json
ccharly Nov 28, 2024
b3e0e26
refactor: remove test:verbose
ccharly Nov 28, 2024
165b65f
fix(tsd): fix tsd support for every new packages
ccharly Nov 28, 2024
e9921c2
fix(keyring-internal-api): fix *.test-d.ts new imports
ccharly Nov 28, 2024
9e5806a
chore: lint + missing toThrow messages in tests
ccharly Nov 28, 2024
744b792
Merge branch 'main' into refactor/split-keyring-api
ccharly Nov 28, 2024
8797cbf
fix(keyring-api): disable no-redundant-type-constituents lint warning…
ccharly Nov 28, 2024
11541a9
chore: better comments
ccharly Nov 28, 2024
b85bfaa
fix: fix main file for new packages
ccharly Nov 28, 2024
d4f18e4
refactor: cleanup deps
ccharly Nov 29, 2024
e9c0e4e
refactor: keyring-internal-api/src/{btc,eth,sol} -> keyring-api/src
ccharly Nov 29, 2024
4ebf2ef
fix: fix imports
ccharly Nov 29, 2024
b250745
refactor: exclude sub-folders index.ts from coverage
ccharly Nov 29, 2024
21ea64a
refactor: remove skipLibCheck for some new packages
ccharly Dec 2, 2024
534a00f
chore: cleanup deps
ccharly Dec 2, 2024
bf6b242
Merge branch 'main' into refactor/split-keyring-api
ccharly Dec 2, 2024
1196b3a
chore: use same @metamask/utils in every new packages
ccharly Dec 2, 2024
72196d6
fix(keyring-api): fix coverage for pagination.ts
ccharly Dec 2, 2024
2d537cc
test: remove jest workaround
ccharly Dec 3, 2024
5f63eea
chore: update new packages descriptions
ccharly Dec 3, 2024
e564e9e
revert: revert StringNumberStruct fix
ccharly Dec 3, 2024
d16e239
refactor: splitting keyring-snap-client into keyring-snap-internal-cl…
ccharly Dec 4, 2024
72d24aa
fix: fix tsd-test.sh when there is not typing test files
ccharly Dec 4, 2024
4c8d50a
fix(keyring-snap-bridge): try to remove explicit build of the keyring…
ccharly Dec 4, 2024
5143ee2
refactor(keyring-api): src/utils/* -> src/api/
ccharly Dec 4, 2024
18d5db2
build: add tsconfig.scripts.json + use this when invoking ts-node
ccharly Dec 5, 2024
4d643fa
chore: update README
ccharly Dec 5, 2024
bf5d282
refactor(keyring-snap-sdk): remove keyring-internal-api dep
ccharly Dec 5, 2024
a9d60e7
chore: update README
ccharly Dec 5, 2024
7de09bb
refactor: move JsonRpcRequest + cleanup deps
ccharly Dec 5, 2024
4ea9529
chore: update README
ccharly Dec 5, 2024
4d107d3
refactor(keyring-snap-bridge): remove unused dep
ccharly Dec 5, 2024
4903f6a
refactor: remove unused deps
ccharly Dec 5, 2024
7947299
chore: better comment
ccharly Dec 5, 2024
d9ad7cc
refactor: some more cleanup
ccharly Dec 5, 2024
388ed88
fix(scripts): workaround prepare-preview-builds.ts typing issue + upd…
ccharly Dec 5, 2024
f78dac4
chore: fix tsconfig.json
ccharly Dec 5, 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
48 changes: 48 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ module.exports = {
'@typescript-eslint/naming-convention': 'warn',
},
},
// @metamask/keyring-utils
{
files: ['packages/keyring-utils/src/**/*.ts'],
extends: ['@metamask/eslint-config-typescript'],
parserOptions,
rules: {
// TODO: re-lint everything once the migration is done
gantunesr marked this conversation as resolved.
Show resolved Hide resolved
'@typescript-eslint/no-explicit-any': 'off',
},
},
// @metamask/keyring-api
{
files: ['packages/keyring-api/src/**/*.ts'],
Expand All @@ -77,6 +87,18 @@ module.exports = {
'jsdoc/newline-after-description': 'off',
},
},
// @metamask/keyring-internal-api
{
files: ['packages/keyring-internal-api/src/**/*.ts'],
extends: ['@metamask/eslint-config-typescript'],
parserOptions,
rules: {
// FIXME: for some reason, it seems eslint is not able to infere those (this
// works on the original repository, so there might be some side-effects now that
// we are building in a monorepo)
'@typescript-eslint/restrict-template-expressions': 'off',
},
},
// @metamask/keyring-eth-hd
{
files: ['packages/keyring-eth-hd/**/*.js'],
Expand Down Expand Up @@ -169,6 +191,32 @@ module.exports = {
'@typescript-eslint/restrict-template-expressions': 'off',
},
},
// @metamask/keyring-snap-sdk
{
files: ['packages/keyring-snap-sdk/src/**/*.test.ts'],
extends: ['@metamask/eslint-config-typescript'],
parserOptions,
rules: {
// FIXME: for some reason, it seems eslint is not able to infere those (this
// works on the original repository, so there might be some side-effects now that
// we are building in a monorepo)
'@typescript-eslint/restrict-template-expressions': 'off',
},
},
// @metamask/keyring-snap-client
{
files: ['packages/keyring-snap-client/src/**/*.ts'],
extends: ['@metamask/eslint-config-typescript'],
parserOptions,
rules: {
// TODO: re-lint everything once the migration is done
'@typescript-eslint/no-explicit-any': 'off',
// FIXME: for some reason, it seems eslint is not able to infere those (this
// works on the original repository, so there might be some side-effects now that
// we are building in a monorepo)
'@typescript-eslint/restrict-template-expressions': 'off',
},
},
],
rules: {
'jsdoc/match-description': [
Expand Down
26 changes: 25 additions & 1 deletion .syncpackrc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,30 @@
"dependencyTypes": ["!local"],
"dependencies": ["@metamask/keyring-api"],
"pinVersion": "workspace:^"
}
},
{
"label": "use workspace version of the keyring-internal-api",
"dependencyTypes": ["!local"],
"dependencies": ["@metamask/keyring-internal-api"],
"pinVersion": "workspace:^"
},
{
"label": "use workspace version of the keyring-snap-sdk",
"dependencyTypes": ["!local"],
"dependencies": ["@metamask/keyring-snap-sdk"],
"pinVersion": "workspace:^"
},
{
"label": "use workspace version of the keyring-snap-client",
"dependencyTypes": ["!local"],
"dependencies": ["@metamask/keyring-snap-client"],
"pinVersion": "workspace:^"
},
{
"label": "use workspace version of the keyring-utils",
"dependencyTypes": ["!local"],
"dependencies": ["@metamask/keyring-utils"],
"pinVersion": "workspace:^"
},
]
}
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ This repository contains the following packages [^fn1]:
- [`@metamask/eth-snap-keyring`](packages/keyring-snap)
- [`@metamask/eth-trezor-keyring`](packages/keyring-eth-trezor)
- [`@metamask/keyring-api`](packages/keyring-api)
- [`@metamask/keyring-internal-api`](packages/keyring-internal-api)
- [`@metamask/keyring-snap-client`](packages/keyring-snap-client)
ccharly marked this conversation as resolved.
Show resolved Hide resolved
- [`@metamask/keyring-snap-sdk`](packages/keyring-snap-sdk)
- [`@metamask/keyring-utils`](packages/keyring-utils)

<!-- end package list -->

Expand All @@ -34,8 +38,19 @@ linkStyle default opacity:0.5
eth_ledger_bridge_keyring(["@metamask/eth-ledger-bridge-keyring"]);
eth_simple_keyring(["@metamask/eth-simple-keyring"]);
eth_trezor_keyring(["@metamask/eth-trezor-keyring"]);
keyring_internal_api(["@metamask/keyring-internal-api"]);
eth_snap_keyring(["@metamask/eth-snap-keyring"]);
keyring_snap_client(["@metamask/keyring-snap-client"]);
keyring_snap_sdk(["@metamask/keyring-snap-sdk"]);
keyring_utils(["@metamask/keyring-utils"]);
keyring_api --> keyring_utils;
keyring_internal_api --> keyring_api;
eth_snap_keyring --> keyring_api;
eth_snap_keyring --> keyring_snap_client;
eth_snap_keyring --> keyring_snap_sdk;
keyring_snap_client --> keyring_api;
keyring_snap_client --> keyring_snap_sdk;
keyring_snap_sdk --> keyring_api;
```

<!-- end dependency graph -->
Expand Down
12 changes: 7 additions & 5 deletions jest.config.packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ module.exports = {
collectCoverage: true,

// An array of glob patterns indicating a set of files for which coverage information should be collected
collectCoverageFrom: ['./src/**/*.ts', '!./src/**/*.test-d.ts'],
collectCoverageFrom: [
'./src/**/*.ts',
// Ignore typing test files
'!./src/**/*.test-d.ts',
// Ignore index of subdirectories
'!./src/**/*/index.ts',
ccharly marked this conversation as resolved.
Show resolved Hide resolved
],

// The directory where Jest should output its coverage files
coverageDirectory: 'coverage',
Expand Down Expand Up @@ -83,10 +89,6 @@ module.exports = {
// FIXME: For now we do require to build some packages (keyring-api) that is then used
ccharly marked this conversation as resolved.
Show resolved Hide resolved
// in the keyring-snap package. This might be fixed after splitting the keyring-api into
// smaller internal packages!
// TODO: Remove this after the split of the keyring-api
// {
'^@metamask/keyring-api/dist/(.*)$': ['<rootDir>/../keyring-api/dist/$1'],
// }
'^jest-environment-jsdom$': ['<rootDir>/../keyring-api/dist/$1'],
'^@metamask/(.+)$': [
'<rootDir>/../$1/src',
Expand Down
19 changes: 8 additions & 11 deletions packages/keyring-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,20 @@
"test": "yarn test:source && yarn test:types",
"test:clean": "jest --clearCache",
"test:source": "jest && jest-it-up",
"test:types": "tsd",
"test:types": "../../scripts/tsd-test.sh ./src",
ccharly marked this conversation as resolved.
Show resolved Hide resolved
"test:watch": "jest --watch"
},
"dependencies": {
"@metamask/snaps-sdk": "^6.7.0",
"@metamask/keyring-utils": "workspace:^",
"@metamask/superstruct": "^3.1.0",
"@metamask/utils": "^9.3.0",
"@types/uuid": "^9.0.8",
"bech32": "^2.0.0",
"uuid": "^9.0.1",
"webextension-polyfill": "^0.12.0"
"bech32": "^2.0.0"
},
"devDependencies": {
"@lavamoat/allow-scripts": "^3.2.1",
"@lavamoat/preinstall-always-fail": "^2.1.0",
"@metamask/auto-changelog": "^3.4.4",
"@metamask/providers": "^18.1.0",
"@metamask/keyring-utils": "workspace:^",
"@ts-bridge/cli": "^0.6.0",
"@types/jest": "^29.5.12",
"@types/node": "^20.12.12",
Expand All @@ -73,9 +70,6 @@
"typedoc": "^0.25.13",
"typescript": "~5.6.3"
},
"peerDependencies": {
"@metamask/providers": "^18.1.0"
},
"engines": {
"node": "^18.18 || >=20"
},
Expand All @@ -91,6 +85,9 @@
}
},
"tsd": {
"directory": "src"
"directory": "src",
"compilerOptions": {
"composite": "false"
}
ccharly marked this conversation as resolved.
Show resolved Hide resolved
}
}
4 changes: 1 addition & 3 deletions packages/keyring-api/src/api/account.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { object, UuidStruct } from '@metamask/keyring-utils';
ccharly marked this conversation as resolved.
Show resolved Hide resolved
import type { Infer } from '@metamask/superstruct';
import { array, enums, record, string } from '@metamask/superstruct';
import { JsonStruct } from '@metamask/utils';

import { object } from '../superstruct';
import { UuidStruct } from '../utils';

/**
* Supported Ethereum account types.
*/
Expand Down
8 changes: 5 additions & 3 deletions packages/keyring-api/src/api/asset.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import {
object,
selectiveUnion,
StringNumberStruct,
} from '@metamask/keyring-utils';
import type { Infer } from '@metamask/superstruct';
import { literal, string } from '@metamask/superstruct';
import {
Expand All @@ -6,9 +11,6 @@ import {
isPlainObject,
} from '@metamask/utils';

import { object, selectiveUnion } from '../superstruct';
import { StringNumberStruct } from '../utils';

/**
* Fungible asset struct.
*/
Expand Down
32 changes: 32 additions & 0 deletions packages/keyring-api/src/api/balance.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { is } from '@metamask/superstruct';

import { BalanceStruct } from './balance';

describe('BalanceStruct', () => {
it.each([
// Valid
{ balance: { amount: '1.0', unit: 'ETH' }, expected: true },
{ balance: { amount: '0.1', unit: 'BTC' }, expected: true },
// FIXME: Those are not valid for `StringNumberStruct`, but they should be:
danroc marked this conversation as resolved.
Show resolved Hide resolved
// { balance: { amount: '.1', unit: 'gwei' }, expected: true },
// { balance: { amount: '.1', unit: 'wei' }, expected: true },
// { balance: { amount: '1.', unit: 'sat' }, expected: true },
// Missing amount
{ balance: { unit: 'ETH' }, expected: false },
// Missing unit
{ balance: { amount: '1.0' }, expected: false },
// Invalid amount type
{ balance: { amount: 1, unit: 'ETH' }, expected: false },
{ balance: { amount: true, unit: 'ETH' }, expected: false },
{ balance: { amount: null, unit: 'ETH' }, expected: false },
// Invalid unit type
{ balance: { amount: '1.0', unit: 1 }, expected: false },
{ balance: { amount: '1.0', unit: true }, expected: false },
{ balance: { amount: '1.0', unit: null }, expected: false },
])(
'returns $expected for is($balance, BalanceStruct)',
({ balance, expected }) => {
expect(is(balance, BalanceStruct)).toBe(expected);
},
);
});
4 changes: 1 addition & 3 deletions packages/keyring-api/src/api/balance.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { object, StringNumberStruct } from '@metamask/keyring-utils';
import type { Infer } from '@metamask/superstruct';
import { string } from '@metamask/superstruct';

import { object } from '../superstruct';
import { StringNumberStruct } from '../utils';

export const BalanceStruct = object({
amount: StringNumberStruct,
unit: string(),
Expand Down
3 changes: 1 addition & 2 deletions packages/keyring-api/src/api/caip.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { definePattern } from '@metamask/keyring-utils';
import { is, type Infer } from '@metamask/superstruct';

import { definePattern } from '../superstruct';

const CAIP_ASSET_TYPE_REGEX =
/^(?<chainId>(?<namespace>[-a-z0-9]{3,8}):(?<reference>[-_a-zA-Z0-9]{1,32}))\/(?<assetNamespace>[-a-z0-9]{3,8}):(?<assetReference>[-.%a-zA-Z0-9]{1,128})$/u;

Expand Down
29 changes: 29 additions & 0 deletions packages/keyring-api/src/api/export.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { is } from '@metamask/superstruct';

import { KeyringAccountDataStruct } from './export';

describe('KeyringAccountDataStruct', () => {
const sym = Symbol('Unique symbol for testing purposes');
it.each([
// Valid
{ data: { foo: 'bar' }, expected: true },
{ data: { foo: 'bar', bar: 1 }, expected: true },
// Undefined is not allowed in JSON
{ data: { foo: undefined, bar: null }, expected: false },
// Invalid
{ data: 0, expected: false },
{
data: '34a0b893b66e312a8b0f7dc4bc4c7930b67f8823513aff5444fb5c64aa060c5a',
expected: false,
},
{ data: sym, expected: false },
// FIXME: Not sure why this one works, maybe the array is
// mapped as: { 0: 0xdead, 1: 0xbeef, 2: '!' }?
{ data: [0xdead, 0xbeef, '!'], expected: true },
])(
'returns $expected for is($data, KeyringAccountDataStruct)',
({ data, expected }) => {
expect(is(data, KeyringAccountDataStruct)).toBe(expected);
},
);
});
3 changes: 3 additions & 0 deletions packages/keyring-api/src/api/keyring.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/* eslint-disable @typescript-eslint/no-redundant-type-constituents */
// This rule seems to be triggering a false positive on the `KeyringAccount`.

import type { Json } from '@metamask/utils';

import type { KeyringAccount } from './account';
Expand Down
Loading
Loading