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

feat: decorate clients with actions #77

Merged
merged 19 commits into from
Feb 20, 2023
Merged

feat: decorate clients with actions #77

merged 19 commits into from
Feb 20, 2023

Conversation

jxom
Copy link
Member

@jxom jxom commented Feb 18, 2023

After some research, it seems like decorating clients with their respective actions won't add too much to the bundle size. For example, adding public actions onto the public client only added 4kB (gzipped) for the createPublicClient import. I think the trade-off of losing tree-shaking here is okay since we gain a massive DX improvement (no extraneous imports + autocomplete of actions on the client).

Example:

import { createPublicClient, http } from 'viem'
import { mainnet } from 'viem/chains'
-import { getBlockNumber } from 'viem/public'

const client = createPublicClient({
  chain: mainnet,
  transport: http(),
})

- const blockNumber = await getBlockNumber(client)
+ const blockNumber = await client.getBlockNumber()

Tree-shaking escape hatch

If a consumer wants to be pedantic about bundle size, we also provide the escape hatch for them to leverage tree-shaking (the previous approach), using the base client (the base client does not attach any actions) – will need to document this.

import { 
-createPublicClient,
+createClient, 
  http 
} from 'viem'
import { mainnet } from 'viem/chains'
import { getBlockNumber } from 'viem/public'

-const client = createPublicClient({
+const client = createClient({
  chain: mainnet,
  transport: http(),
})

const blockNumber = await getBlockNumber(client)

TODO

  • Public Client + Actions
  • Test Client + Actions
  • Wallet Client + Actions
  • Tests
  • Update examples
  • Entrypoint refactor

@changeset-bot
Copy link

changeset-bot bot commented Feb 18, 2023

🦋 Changeset detected

Latest commit: 91db4b7

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Feb 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
viem-playground ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 20, 2023 at 8:03PM (UTC)
viem-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 20, 2023 at 8:03PM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Feb 18, 2023

Size Change: +2.46 kB (+7%) 🔍

Total Size: 40.1 kB

Filename Size Change
dist/chains.js 1.3 kB -2 B (0%)
dist/chunk-LT3TMHSU.js 0 B -5.74 kB (removed) 🏆
dist/chunk-X5J5H2UR.js 0 B -1.63 kB (removed) 🏆
dist/chunk-XPJ4VYLC.js 0 B -21 kB (removed) 🏆
dist/contract.js 475 B +18 B (+4%)
dist/ens.js 355 B -1.44 kB (-80%) 🏆
dist/index.js 4.29 kB +1.61 kB (+60%) 🆘
dist/public.js 473 B +7 B (+2%)
dist/test.js 518 B -856 B (-62%) 🏆
dist/utils/index.js 868 B -1 B (0%)
dist/wallet.js 295 B +1 B (0%)
dist/chunk-5ULIWMK3.js 1.72 kB +1.72 kB (new file) 🆕
dist/chunk-GFTOFJQR.js 21 kB +21 kB (new file) 🆕
dist/chunk-IVWK2BXY.js 5.75 kB +5.75 kB (new file) 🆕
dist/chunk-KAPCXVQA.js 1.38 kB +1.38 kB (new file) 🆕
dist/chunk-YL2PFU3G.js 1.63 kB +1.63 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size
dist/window.js 67 B

compressed-size-action

@codecov
Copy link

codecov bot commented Feb 19, 2023

Codecov upload limit reached ⚠️

This org is currently on the free Basic Plan; which includes 250 free private repo uploads each rolling month. This limit has been reached and additional reports cannot be generated. For unlimited uploads, upgrade to our pro plan.

Do you have questions or need help? Connect with our sales team today at [email protected]

Copy link
Member

@tmm tmm left a comment

Choose a reason for hiding this comment

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

Looks good. Few docs suggestions. Also, wired up some missing generics in 318770c

Comment on lines +33 to +36
::: warning
A name must be [normalized via UTS-46 normalization](https://docs.ens.domains/contract-api-reference/name-processing) before being used with `getEnsAddress`.
This can be achieved by using the `normalize` utility.
:::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
::: warning
A name must be [normalized via UTS-46 normalization](https://docs.ens.domains/contract-api-reference/name-processing) before being used with `getEnsAddress`.
This can be achieved by using the `normalize` utility.
:::
::: warning
Since ENS names prohibit certain forbidden characters (e.g. underscore) and have other validation rules, you likely want to [normalize ENS names](https://docs.ens.domains/contract-api-reference/name-processing#normalising-names) with [UTS-46 normalization](https://unicode.org/reports/tr46) before passing them to `getEnsAddress`.
This can be achieved by using viem's built-in [`normalize`](/docs/ens/utilities/normalize) utility.
:::

Comment on lines +20 to +23
::: warning
A label must be [normalized via UTS-46 normalization](https://docs.ens.domains/contract-api-reference/name-processing) before being hashed with labelhash.
This can be achieved by using the `normalize` utility.
:::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
::: warning
A label must be [normalized via UTS-46 normalization](https://docs.ens.domains/contract-api-reference/name-processing) before being hashed with labelhash.
This can be achieved by using the `normalize` utility.
:::
::: warning
Since ENS names prohibit certain forbidden characters (e.g. underscore) and have other validation rules, you likely want to [normalize labels](https://docs.ens.domains/contract-api-reference/name-processing#normalising-names) with [UTS-46 normalization](https://unicode.org/reports/tr46) before passing them to `labelhash`.
This can be achieved by using viem's built-in [`normalize`](/docs/ens/utilities/normalize) utility.
:::

Comment on lines +20 to +23
::: warning
A name must be [normalized via UTS-46 normalization](https://docs.ens.domains/contract-api-reference/name-processing) before being hashed with namehash.
This can be achieved by using the `normalize` utility.
:::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
::: warning
A name must be [normalized via UTS-46 normalization](https://docs.ens.domains/contract-api-reference/name-processing) before being hashed with namehash.
This can be achieved by using the `normalize` utility.
:::
::: warning
Since ENS names prohibit certain forbidden characters (e.g. underscore) and have other validation rules, you likely want to [normalize ENS names](https://docs.ens.domains/contract-api-reference/name-processing#normalising-names) with [UTS-46 normalization](https://unicode.org/reports/tr46) before passing them to `namehash`.
This can be achieved by using viem's built-in [`normalize`](/docs/ens/utilities/normalize) utility.
:::

} from './utils/ens'

export { normalize } from './utils/ens/normalize'
Copy link
Member

Choose a reason for hiding this comment

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

For tree shake-ability?

Comment on lines +13 to +17
AddChainArgs,
FormattedTransactionRequest,
GetPermissionsResponse,
GetAccountsResponse,
RequestAccountsResponse,
Copy link
Member

Choose a reason for hiding this comment

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

Added a few missing args/response types for consistency

Base automatically changed from jxom/watch-event-fallback to main February 20, 2023 19:52
@jxom jxom merged commit d6a29f5 into main Feb 20, 2023
@jxom jxom deleted the jxom/actions-on-clients branch February 20, 2023 20:01
@github-actions github-actions bot mentioned this pull request Feb 20, 2023
jxom added a commit that referenced this pull request Feb 20, 2023
* feat: fall back to getLogs if filters are not supported

* Update src/actions/public/watchContractEvent.test.ts

Co-authored-by: awkweb <[email protected]>

* pr review

* feat: decode event log topics & data

* wip: public

* wip: add ens

* docs: public actions

* wip: wallet actions

* docs: wallet actions

* wip: test actions

* wip: add utils to main entrypoint

* tests: add tests

* playgrounds: update

* chore: changeset

* test

* refactor: wire up decorator generics

* fix conflicts

* format

---------

Co-authored-by: awkweb <[email protected]>
jxom added a commit that referenced this pull request Feb 20, 2023
* feat: add timeout as config to transport

* chore: changeset

* feat: decode log `topics` + `data` & infer types for logs (#75)

* feat: decode event log topics & data

* docs: createContractEventFilter

* pr review

* feat: fall back to `getLogs` if filters are not supported (#76)

* feat: fall back to getLogs if filters are not supported

* Update src/actions/public/watchContractEvent.test.ts

Co-authored-by: awkweb <[email protected]>

* pr review

---------

Co-authored-by: awkweb <[email protected]>

* feat: decorate clients with actions (#77)

* feat: fall back to getLogs if filters are not supported

* Update src/actions/public/watchContractEvent.test.ts

Co-authored-by: awkweb <[email protected]>

* pr review

* feat: decode event log topics & data

* wip: public

* wip: add ens

* docs: public actions

* wip: wallet actions

* docs: wallet actions

* wip: test actions

* wip: add utils to main entrypoint

* tests: add tests

* playgrounds: update

* chore: changeset

* test

* refactor: wire up decorator generics

* fix conflicts

* format

---------

Co-authored-by: awkweb <[email protected]>

---------

Co-authored-by: awkweb <[email protected]>
@github-actions github-actions bot mentioned this pull request Mar 6, 2023
@jxom jxom mentioned this pull request Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants