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

feat: Generic EngineTypes.RequestParams #3415

Open
wants to merge 2 commits into
base: v2.0
Choose a base branch
from

Conversation

huck-hgraph
Copy link

Description

When creating session request payload, it would be nice to let the developer create strongly typed RequestParams. This can easily be done be converting the existing EngineTypes.RequestParams to a TypeScript generic, with a default type parameter value of any. The default value makes the type parameter optional so that any existing usage of the type will not change, but gives the developer the option to add more specific params types if they want to.

Type of change

  • Chore (non-breaking change that addresses non-functional tasks, maintenance, or code quality improvements)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Draft PR (breaking/non-breaking change which needs more work for having a proper functionality [Mark this PR as ready to review only when completely ready])
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How has this been tested?

Ran the build command in the root repo successfully.

Examples/Screenshots (Optional)

import { EngineTypes } from "@walletconnect/types";

type TransactionParams = {
  transaction: {
    type: string;
    bytes: string;
  };
};

type DefaultRequestParams = EngineTypes.RequestParams;
type TypedRequestParams = EngineTypes.RequestParams<TransactionParams>;

const untypedParamsPayload: DefaultRequestParams = {
  chainId: "foo:testnet",
  topic: session!.topic,
  request: {
    method: "testMethod",
    params: { // by default, params are `any` type, so anything could go here
      untyped: {
        data: "isDangerous",
      },
    },
  },
};

const typedParamsPayload: TypedRequestParams = {
  chainId: "foo:testnet",
  topic: session!.topic,
  request: {
    method: "testMethod",
    params: { // using the generic type parameter forces the params to adhere to a specific interface
      transaction: {
        type: "CryptoTransfer",
        bytes: "Cxf0d...",
      },
    },
  },
};

Checklist

  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Additional Information (Optional)

For reference, I wanted to have a "TypedRequestParams" type when adding the Hedera example to the web-examples/dapps/react-dapp-v2, and this is how that can be accomplished with the fixed EngineTypes.RequestParams as a base type. Definitely not as readable/friendly as making that type generic:

type TypedRequestParams<T> = Omit<EngineTypes.RequestParams, "request"> & {
  request: Omit<EngineTypes.RequestParams["request"], "params"> & {
    params: T;
  };
};

Copy link
Member

@ganchoradkov ganchoradkov left a comment

Choose a reason for hiding this comment

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

Hey @huck-hgraph thanks for the PR, please update the branch to latest and we can merge

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.

4 participants