-
Notifications
You must be signed in to change notification settings - Fork 36
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
🧼 cleaning out ethers from utils/tests/bytes
#237
Conversation
const inputs: ReadonlyArray<BytesLikeWithNumber[]> = [[0, 1]]; | ||
|
||
inputs.forEach((input) => { | ||
expect(concat(input)).toStrictEqual(ethers.utils.concat(input as any)); | ||
expect(concat(input)).toStrictEqual(new Uint8Array([0, 1])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great type improvement ✅
it('should match expected slice - hexadecimal strings', () => { | ||
const hexValues = ['0x123456']; | ||
hexValues.forEach((hexValue) => { | ||
expect(hexDataSlice(hexValue, 0, 2)).toStrictEqual('0x1234'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dawsbot mmmm trying to replicate this on my machine. I'll just be careful for now till I have time to git this linter in my IDE on my own time.
expect(() => hexlify(value as BytesLike)).toThrow( | ||
'invalid hexlify value', | ||
); | ||
expect(() => hexlify(value as BytesLike)).toThrow('invalid hexlify value'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to never type-cast like this. Instead we should prefer typing at the point of defining the value. Here's why:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok! so as a general rule of thumb never bypass the type checker.
@dawsbot ik you like prs to be shorter, but these changes are very simple. I thought it would just be easier to put them in one pr.
![Screen Shot 2023-05-21 at 7 03 34 PM](https://private-user-images.githubusercontent.com/106350168/239772283-c657b00a-c291-4ef8-bf50-76be36cdbbc1.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjE1MzU1NDUsIm5iZiI6MTcyMTUzNTI0NSwicGF0aCI6Ii8xMDYzNTAxNjgvMjM5NzcyMjgzLWM2NTdiMDBhLWMyOTEtNGVmOC1iZjUwLTc2YmUzNmNkYmJjMS5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNzIxJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDcyMVQwNDE0MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT0xYTZkMTVlOGQ1Zjk2MDg5MGJiYmUxZjIzZGVkNDY1NDk5MTMxN2ExZjg4ODk2NzFhMTdiNTIzZDI3ZmY1NTkxJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.41SmknkzjNKWKQch4XxtbYSAxOYtO2gH1cCZi97wPzE)