Skip to content

Conversation

@Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Dec 12, 2022

This is a prerequisite for #1014.

I've moved all types from snaps-types to snaps-utils, to avoid circular dependencies. snaps-types has also been refactored to a "regular" package, rather than a type-only package. Even though it still only contains types, this was required for it to be able to import from other monorepo packages.

Breaking changes

  • OnRpcRequestHandler, OnTransactionResponse, OnTransactionHandler, and OnCronjobHandler were using unknown[] | Record<string, unknown>. Since values need to be JSON-compatible, I've changed this to Json[] | Record<string, Json>.
  • SnapRpcHandler was removed. This seemed like a legacy type, and was the same as OnRpcRequestHandler.
  • global.d.ts in snaps-types was removed, as it is no longer necessary. snaps-types creates the globals when the package itself is imported, meaning that the following works now:
    import { OnRpcRequestHandler } from '@metamask/snaps-types';
    
    // Since `@metamask/snaps-types` is imported, the `snap` global is declared.
    snap.request();
  • SnapFunctionExports and SnapExports are no longer exported from snaps-types.

Closes #1054.
Closes #815.

@Mrtenz Mrtenz marked this pull request as ready for review December 12, 2022 11:42
@Mrtenz Mrtenz requested a review from a team as a code owner December 12, 2022 11:42
@FrederikBolding FrederikBolding self-requested a review December 12, 2022 13:17
@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2022

Codecov Report

Merging #1060 (636d095) into main (f39d07e) will decrease coverage by 1.16%.
The diff coverage is 5.30%.

@@            Coverage Diff             @@
##             main    #1060      +/-   ##
==========================================
- Coverage   93.52%   92.36%   -1.17%     
==========================================
  Files          95       96       +1     
  Lines        9922    10044     +122     
  Branches      941      942       +1     
==========================================
- Hits         9280     9277       -3     
- Misses        642      767     +125     
Impacted Files Coverage Δ
packages/snaps-utils/src/handlers.ts 0.00% <0.00%> (ø)
...cution-environments/src/common/BaseSnapExecutor.ts 92.12% <100.00%> (+0.01%) ⬆️
...cution-environments/src/common/endowments/index.ts 100.00% <100.00%> (ø)
...snaps-execution-environments/src/common/keyring.ts 100.00% <100.00%> (ø)
...ps-execution-environments/src/common/validation.ts 100.00% <100.00%> (ø)
packages/snaps-utils/src/types.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Mrtenz Mrtenz force-pushed the mrtenz/snaps-types-refactor branch from 9cf588d to 636d095 Compare December 13, 2022 11:08
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

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

Needs a rebase but otherwise lgtm

@Mrtenz Mrtenz merged commit b3ce1ff into main Dec 13, 2022
@Mrtenz Mrtenz deleted the mrtenz/snaps-types-refactor branch December 13, 2022 12:43
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.

Move internal snaps-types types to snaps-utils Re-export MetaMask types from snap-types

4 participants