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

fix: .extend performance #2328

Merged
merged 6 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 0 additions & 2 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
### Description

Comment on lines -1 to -2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this lol

<!-- What is this PR solving? Write a clear description or reference the issues it solves (e.g. `fixes #123`). What other alternatives have you explored? Are there any parts you think require more attention from reviewers? -->

<!----------------------------------------------------------------------
Expand Down
17 changes: 11 additions & 6 deletions .github/workflows/verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
timeout-minutes: 5
strategy:
matrix:
typescript-version: ['5.3.2', 'latest']
version: ['5.0.4', '5.1.6', '5.2.2', '5.3.3', '5.4.5']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test against more ts versions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if it's worth the work to fix the types that are incompatible with older versions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed. on it!


steps:
- name: Clone repository
Expand All @@ -65,7 +65,7 @@ jobs:
- name: Install dependencies
uses: ./.github/actions/install-dependencies

- run: bun i -d typescript@${{ matrix.typescript-version }}
- run: bun i -d typescript@${{ matrix.version }}

- name: Build contracts
shell: bash
Expand All @@ -74,10 +74,15 @@ jobs:
- name: Check types
run: bun run typecheck

- name: Test types
run: bun run test:typecheck
env:
VITE_ANVIL_BLOCK_NUMBER: ${{ vars.VITE_ANVIL_BLOCK_NUMBER }}
- name: Bench types
run: bun run typebench

# Redundant with `pnpm typecheck`
# If Vitest adds special features in the future, e.g. type coverage, can add this back!
# - name: Test types
# run: bun run test:typecheck
# env:
# VITE_ANVIL_BLOCK_NUMBER: ${{ vars.VITE_ANVIL_BLOCK_NUMBER }}

test:
name: Test
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ _esm
_types
*.local
.DS_Store
.attest
.eslintcache
.next
bench
Expand Down
12 changes: 12 additions & 0 deletions .size-limit.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,17 @@
"path": "./src/_esm/ens/index.js",
"limit": "19 kB",
"import": "{ getEnsAvatar }"
},
{
"name": "viem/siwe",
"path": "./src/_esm/siwe/index.js",
"limit": "28 kB",
"import": "*"
},
{
"name": "viem/siwe (tree-shaking)",
"path": "./src/_esm/siwe/index.js",
"limit": "26 kB",
"import": "{ verifySiweMessage }"
}
]
Binary file modified bun.lockb
Binary file not shown.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@
"test:env:vite": "cd environments/vite && bun run test",
"test:typecheck": "SKIP_GLOBAL_SETUP=true vitest --typecheck.only -c ./test/vitest.config.ts",
"test:ui": "vitest dev -c ./test/vitest.config.ts --ui",
"typebench": "tsx test/typebench.ts --benchPercentThreshold 10 --benchErrorOnThresholdExceeded",
"typecheck": "tsc --noEmit",
"vectors": "bun test vectors/**/*.test.ts",
"vectors:generate": "bun vectors/generate.ts",
"version:update": "bun scripts/updateVersion.ts"
},
"devDependencies": {
"@actions/core": "^1.10.0",
"@actions/github": "^5.1.1",
"@arktype/attest": "0.7.5",
"@biomejs/biome": "^1.7.3",
"@changesets/changelog-github": "^0.4.5",
"@changesets/cli": "^2.23.2",
Expand All @@ -63,6 +63,7 @@
"rimraf": "^4.4.1",
"simple-git-hooks": "^2.8.1",
"size-limit": "^11.1.2",
"tsx": "^4.11.0",
"typescript": "5.4.2",
"vite": "^5.0.7",
"vitest": "^1.0.4"
Expand Down
1 change: 0 additions & 1 deletion src/actions/public/multicall.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,6 @@ test('MulticallParameters', async () => {
| readonly [`0x${string}`]
| readonly [`0x${string}`, `0x${string}`]
| undefined
value?: undefined
}>()
})

Expand Down
16 changes: 16 additions & 0 deletions src/clients/createPublicClient.bench-d.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these type benchmarking tests make sure the types don't blow up too much

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { bench } from '@arktype/attest'

import { createClient } from './createClient.js'
import { createPublicClient } from './createPublicClient.js'
import { publicActions } from './decorators/public.js'
import { http } from './transports/http.js'

bench('createPublicClient', () => {
const client = createPublicClient({ transport: http() })
return {} as typeof client
}).types([13470, 'instantiations'])

bench('createClient.extend + publicActions', () => {
const client = createClient({ transport: http() }).extend(publicActions)
return {} as typeof client
}).types([246356, 'instantiations'])
16 changes: 16 additions & 0 deletions src/clients/createWalletClient.bench-d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { bench } from '@arktype/attest'

import { createClient } from './createClient.js'
import { createWalletClient } from './createWalletClient.js'
import { walletActions } from './decorators/wallet.js'
import { http } from './transports/http.js'

bench('createWalletClient', () => {
const client = createWalletClient({ transport: http() })
return {} as typeof client
}).types([1384, 'instantiations'])

bench('createClient.extend + walletActions', () => {
const client = createClient({ transport: http() }).extend(walletActions)
return {} as typeof client
}).types([193153, 'instantiations'])
42 changes: 38 additions & 4 deletions src/experimental/eip5792/actions/writeContracts.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import type { AbiStateMutability, Narrow } from 'abitype'
import type { Abi, AbiStateMutability, Address, Narrow } from 'abitype'

import type { Client } from '../../../clients/createClient.js'
import type { Transport } from '../../../clients/transports/createTransport.js'
import type { ErrorType } from '../../../errors/utils.js'
import type { Account, GetAccountParameter } from '../../../types/account.js'
import type { Chain, GetChainParameter } from '../../../types/chain.js'
import type { ContractFunctionParameters } from '../../../types/contract.js'
import type {
ContractFunctionArgs,
ContractFunctionName,
GetValue,
UnionWiden,
Widen,
} from '../../../types/contract.js'
import type { MulticallContracts } from '../../../types/multicall.js'
import {
type EncodeFunctionDataErrorType,
Expand All @@ -19,7 +25,8 @@ import {
} from './sendCalls.js'

export type WriteContractsParameters<
contracts extends readonly unknown[] = readonly ContractFunctionParameters[],
contracts extends
readonly unknown[] = readonly WriteContractFunctionParameters[],
chain extends Chain | undefined = Chain | undefined,
account extends Account | undefined = Account | undefined,
chainOverride extends Chain | undefined = Chain | undefined,
Expand Down Expand Up @@ -98,7 +105,7 @@ export async function writeContracts<
chainOverride
>,
): Promise<WriteContractsReturnType> {
const contracts = parameters.contracts as ContractFunctionParameters[]
const contracts = parameters.contracts as WriteContractFunctionParameters[]
const calls = contracts.map((contract) => {
const { address, abi, functionName, args, value } = contract
return {
Expand All @@ -113,3 +120,30 @@ export async function writeContracts<
})
return sendCalls(client, { ...parameters, calls } as SendCallsParameters)
}

export type WriteContractFunctionParameters<
abi extends Abi | readonly unknown[] = Abi,
mutability extends AbiStateMutability = AbiStateMutability,
functionName extends ContractFunctionName<
abi,
mutability
> = ContractFunctionName<abi, mutability>,
args extends ContractFunctionArgs<
abi,
mutability,
functionName
> = ContractFunctionArgs<abi, mutability, functionName>,
///
allFunctionNames = ContractFunctionName<abi, mutability>,
allArgs = ContractFunctionArgs<abi, mutability, functionName>,
// when `args` is inferred to `readonly []` ("inputs": []) or `never` (`abi` declared as `Abi` or not inferrable), allow `args` to be optional.
// important that both branches return same structural type
> = {
address: Address
abi: abi
functionName:
| allFunctionNames // show all options
| (functionName extends allFunctionNames ? functionName : never) // infer value
args?: (abi extends Abi ? UnionWiden<args> : never) | allArgs | undefined
} & (readonly [] extends allArgs ? {} : { args: Widen<args> }) &
GetValue<abi, functionName>
Comment on lines +124 to +149
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated this type here for now. will address during some more extensive cleanup.

1 change: 1 addition & 0 deletions src/experimental/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export {
type WriteContractsErrorType,
type WriteContractsParameters,
type WriteContractsReturnType,
type WriteContractFunctionParameters,
writeContracts,
} from './eip5792/actions/writeContracts.js'
export {
Expand Down
4 changes: 1 addition & 3 deletions src/types/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,7 @@ export type ContractFunctionParameters<
| allFunctionNames // show all options
| (functionName extends allFunctionNames ? functionName : never) // infer value
args?: (abi extends Abi ? UnionWiden<args> : never) | allArgs | undefined
} & (readonly [] extends allArgs ? {} : { args: Widen<args> }) &
// TODO: Remove `GetValue` from here (should be applied to top-level type as separate utility)
GetValue<abi, functionName>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetValue was causing ContractFunctionParameters to blow up when used with .extend + publicActions

} & (readonly [] extends allArgs ? {} : { args: Widen<args> })

export type ContractFunctionReturnType<
abi extends Abi | readonly unknown[] = Abi,
Expand Down
7 changes: 7 additions & 0 deletions test/typebench.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { globby } from 'globby'
import { tsImport } from 'tsx/esm/api'

const paths = await globby(['src/**/*.bench-d.ts'])
for (const path of paths) {
await tsImport(`../${path}`, import.meta.url)
}
8 changes: 0 additions & 8 deletions vercel.json
tmm marked this conversation as resolved.
Show resolved Hide resolved

This file was deleted.