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

Update TypeScript to latest stable version #28

Merged
merged 1 commit into from
Nov 3, 2022
Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Oct 26, 2022

TypeScript has been updated to the latest stable version. Our standard module TypeScript configuration has been brought over from the module template as well, including a separate configuration file for production builds (as opposed to the root file used for things like tests).

A build:clean has been added as well, for consistency with the module template.

@Gudahtt
Copy link
Member Author

Gudahtt commented Oct 26, 2022

This depends upon #26

@Gudahtt Gudahtt force-pushed the update-lint-configuration branch from 5770eca to f990397 Compare October 26, 2022 05:49
@Gudahtt Gudahtt force-pushed the update-typescript branch 2 times, most recently from e310b58 to 49a8d6a Compare October 26, 2022 05:52
@Gudahtt Gudahtt force-pushed the update-lint-configuration branch from f990397 to 98dd4b4 Compare October 26, 2022 13:48
@Gudahtt Gudahtt force-pushed the update-typescript branch 2 times, most recently from a7ee58b to 1fd23bc Compare October 26, 2022 13:53
@Gudahtt Gudahtt force-pushed the update-lint-configuration branch 2 times, most recently from 214c6b9 to 4325f98 Compare October 31, 2022 21:43
@Gudahtt Gudahtt force-pushed the update-lint-configuration branch 2 times, most recently from 96f24c9 to 26b3318 Compare November 3, 2022 20:03
Base automatically changed from update-lint-configuration to main November 3, 2022 20:11
TypeScript has been updated to the latest stable version. Our standard
module TypeScript configuration has been brought over from the module
template as well, including a separate configuration file for
production builds (as opposed to the root file used for things like
tests).

A `build:clean` has been added as well, for consistency with the module
template.
@Gudahtt Gudahtt marked this pull request as ready for review November 3, 2022 20:18
@Gudahtt Gudahtt requested a review from a team as a code owner November 3, 2022 20:18
Comment on lines 274 to 280
export function serializeBufferForStorage(buffer: Uint8Array): string {
let result = '0x';
const len = buffer.length || buffer.byteLength;
for (let i = 0; i < len; i++) {
result += unprefixedHex(buffer[i]);
}
buffer.forEach((value) => {
result += unprefixedHex(value);
});
return result;
}
Copy link
Member

Choose a reason for hiding this comment

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

This entire function could be replaced with bytesToHex from @metamask/utils.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice! I'm tempted to leave this inline for now until we migrate this into the monorepo, to avoid the dependency bloat problem of having incompatible util library versions (the monorepo basically solves this by making it possible for them to be kept in sync). I'll create an issue for this though, thanks for pointing it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

#30

Copy link
Member

Choose a reason for hiding this comment

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

We could make it a peer dependency if you don't want dependency bloat? The version in the extension has the bytes functions already, a lot of functions in this library could be replaced.

Copy link
Member Author

@Gudahtt Gudahtt Nov 3, 2022

Choose a reason for hiding this comment

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

The dependency bloat problem I had in mind was where we need multiple different copies due to major version updates of the utils package, which leaves us with multiple copies unless we're very diligent with updating all packages that use utils (sometimes with can take up to 4 levels of coordinated releases). Adding it as a peer dependency just moves that problem up one level. With the monorepo we are planning on using a Yarn constraint to ensure we have compatible versions across the board.

I've been holding off on replacing internal functions with utils across various different packages for this reason. The monorepo should land tomorrow or Monday though, and I am hoping that migrating a package into it will be a straightforward process. So we may be able to migrate this into the monorepo as early as next week.

@Gudahtt Gudahtt merged commit e66f3ca into main Nov 3, 2022
@Gudahtt Gudahtt deleted the update-typescript branch November 3, 2022 20:30
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