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

Provide TypeScript type definitions #122

Open
frangio opened this issue May 15, 2020 · 12 comments
Open

Provide TypeScript type definitions #122

frangio opened this issue May 15, 2020 · 12 comments

Comments

@frangio
Copy link
Contributor

frangio commented May 15, 2020

Requested in OpenZeppelin/openzeppelin-test-environment#73 (comment).

Some prior conversation in #116 (comment).

@frangio
Copy link
Contributor Author

frangio commented May 15, 2020

@bruce-plutusds is there a chance we could see your port of this library to TypeScript?

@frangio
Copy link
Contributor Author

frangio commented May 18, 2020

I tried running TypeScript to generate type declarations from our current JavaScript files, but it's triggering a compiler bug, and it doesn't seem feasible to fix it. So we will have to migrate everything to TypeScript. We can do this in two steps:

  1. Migrate all source files to TypeScript, with generous use of any. Keep the tests in JavaScript for now, since the priority is getting type definitions for the public API.
  2. Progressively remove uses of any. Remember that the priority is typing the public API, and internal functions are not as important (for now).

Things to watch out for:

  • Truffle and Web3 receipts, which we use in expectEvent. I'm not sure if those packages provide their own types. However, since our library receives the receipt objects from the user, and we use very few properties anyway, I'd suggest we define our own interfaces with the properties we need.
  • Numbers. Functions that receive numbers may work with BN, number, or even string. We have to check and type appropriately.

@nventuro
Copy link
Contributor

Are there any plans to tackle this soon? 😁

@frangio
Copy link
Contributor Author

frangio commented Sep 14, 2020

@nventuro Sorry we don't have the time to tackle this right now, but would welcome and review any PRs. 🙂

@omarish
Copy link

omarish commented Nov 14, 2020

Big fan of this library. I started a PR to add type definitions: #141.

@nowtryz
Copy link

nowtryz commented Apr 20, 2021

Hey! Any news on this one?

@frangio
Copy link
Contributor Author

frangio commented Apr 20, 2021

No news, sorry.

@omarish
Copy link

omarish commented Apr 20, 2021

+1. Haven't had much time to continue this. Happy to pass the torch to someone else if they want to run with it.

@codler
Copy link

codler commented May 30, 2021

Hope that base configuration can be integrated, then other people can convert partially

@FrozenKiwi
Copy link

I think I can spare the bandwidth to finish this off, provided the scope is set in advance.

OS dev is frustrating, and so is submitting PR's that immediately fall by the way because they don't check box (X) :-)

@robercano
Copy link

Are there any plans on tackling this? Anybody doing the port? Thanks!

@stoooops
Copy link

stoooops commented Mar 16, 2023

I also ran into this trying to test events so I added:

// types/openzeppelin-test-helpers.d.ts
declare module '@openzeppelin/test-helpers' {
  import { ContractReceipt } from 'ethers'

  export function expectEvent(receipt: ContractReceipt, eventName: string, eventArgs: Record<string, any>): void
}

which gets picked up by default via tsc (typeRoots setting in tsconfig) and allowed me to use

import { expectEvent } from '@openzeppelin/test-helpers'

in my .test.ts TypeScript test files.

But strangely it was throwing errors that it couldn't find events which were clearly there due to my hand-written test code that validated it.

So ultimately I decided to remove the dependency on @openzeppelin/test-helpers and write my own helper function for testing events:

import { ContractReceipt } from "@ethersproject/contracts";
import { EventFilter } from "@ethersproject/abstract-provider";
import { expect } from "chai";

// Asserts that the logs in receipt contain an event with name eventName and arguments
// that match those specified in eventArgs.
function expectEvent(
  receipt: ContractReceipt,
  eventName: string,
  eventArgs: Record<string, any>
) {
  expect(receipt.events).to.not.be.undefined;

  const event = receipt.events?.find((e) => e.event === eventName);
  expect(event, `Event '${eventName}' not found`).to.not.be.undefined;

  for (const [key, value] of Object.entries(eventArgs)) {
    expect(event?.args?.[key], `Event argument '${key}' not found`).to.equal(value);
  }
}

// Asserts that the logs in receipt contain a single event with name eventName and arguments
// that match those specified in eventArgs.
function expectOneEvent(receipt: ContractReceipt, eventName: string, eventArgs: Record<string, any>) {
  expect(receipt.events).to.not.be.undefined
  expect(receipt.events?.length, `Expected exactly 1 event`).to.equal(1)

  expectEvent(receipt, eventName, eventArgs)
}

which readers are welcome to use or adapt to their use case.

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

No branches or pull requests

8 participants