Skip to content

Conversation

gabrocheleau
Copy link
Contributor

TypeScript 4.1 introduced template literal types: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-1.html

We can use those to restrict the type of some strings further with no runtime performance downside. This PR updates PrefixedHexString from string to 0x{string}

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

Attention: Patch coverage is 94.44444% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 84.88%. Comparing base (4be68d2) to head (9353778).
Report is 7 commits behind head on master.

❗ Current head 9353778 differs from pull request most recent head f235ea7. Consider uploading reports for the commit f235ea7 to get more accurate results

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block ?
blockchain ?
client 84.88% <94.44%> (-0.04%) ⬇️
common ?
devp2p ?
evm ?
genesis ?
statemanager ?
trie ?
tx ?
util ?
vm ?
wallet ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@gabrocheleau gabrocheleau marked this pull request as draft April 8, 2024 13:39
@gabrocheleau gabrocheleau added PR state: WIP type: enhancement target: master Work to be done towards master branch labels Apr 8, 2024
@holgerd77
Copy link
Member

Oh, that’s actually a hyper-great one, love this! 🤩🙏❤️

@holgerd77
Copy link
Member

holgerd77 commented Apr 9, 2024

(feel that I should really start looking into TypeScript (updates) a bit more, maybe/likely there is other new base functionality we can leverage in a useful and very targeted way. Also, we can actually generally look into updating our Typescript version (what is a bit tricky and what I haven't given much thought yet: what is actually with TypeScript backwards compatibility, so if we update to a new version and use some of the very latest features. Then we likely also break other people's TypeScript code. 🤔 So guess this also needs at least some basic thinking and following guidelines we set for ourselves (maybe in a first round: just not get too hyper-progressive with this stuff and not fully jump on the latest stuff from 2 weeks ago (atm we are not in danger with our 4.7.4 usage from (just googled...) mid 2022. 😂)).

@gabrocheleau gabrocheleau marked this pull request as ready for review April 15, 2024 22:26
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants