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

util: update prefixedHexString to use literal string template #3348

Merged
merged 14 commits into from
Apr 16, 2024

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

@acolytec3 acolytec3 merged commit d210d7c into master Apr 16, 2024
34 checks passed
@holgerd77 holgerd77 deleted the util/prefixed-hex-string-type branch April 16, 2024 13:22
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