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

ethersproject 5.5.0 BaseProvider ovewrite Provider failed #2190

Closed
hogan0520 opened this issue Oct 20, 2021 · 16 comments
Closed

ethersproject 5.5.0 BaseProvider ovewrite Provider failed #2190

hogan0520 opened this issue Oct 20, 2021 · 16 comments
Labels
bug Verified to be an issue.

Comments

@hogan0520
Copy link

BaseProvider => resolveName(name: string | Promise): Promise<null | string>;
Provider => resolveName(name: string | Promise): Promise;

@hogan0520 hogan0520 added the investigate Under investigation and may be a bug. label Oct 20, 2021
@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

Good catch. Thanks.

Did you get a TypeScript error? Are there any others?

@Lumersgo
Copy link

same here when build with ether package

@hogan0520
Copy link
Author

Good catch. Thanks.

Did you get a TypeScript error? Are there any others?

got a build error in Typescript

@holgerd77
Copy link

Not sure if related, but just to drop here: our Hardhat E2E tests on EthereumJS are currently also failing with some Ethers BaseProvider-mentioning errors.

@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

@holgerd77 Is it using 5.5.0 too? I'll get a fixed release asap.

@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

What version of TypeScript are you using?

I have made the changes, but would feel more comfortable if I could get TypeScript to emit errors; it is blindly accepting it. :(

@ricmoo ricmoo added bug Verified to be an issue. on-deck This Enhancement or Bug is currently being worked on. and removed investigate Under investigation and may be a bug. labels Oct 20, 2021
@hogan0520
Copy link
Author

image
image
image
image
Typescript: 3.9
Return type error function => resolveName, lookupAddress

@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

Can you try using TypeScript 4.3 or 4.4?

It's weird it is complaining about the .on, because I don't think I changed that...

@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

Oh! Actually, the .on and related errors are probably related to the previous errors. Since the previous errors determine that this is not a valid sub-class of Provider, they fail to allow a return type of this to match Provider.

The new version is passing through the CI. I'll ping you once I've published it...

@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

I've published 5.5.1 if you can try it out?

@jeffreyzzzs
Copy link

#2181

@holgerd77
Copy link

@ricmoo I can't really see to what Ethers version dependency installation resolves in the CI, since it is just some subdependency for Hardhat. We are using TypeScript v4.4.2.

I've triggered a new dummy CI run here ethereumjs/ethereumjs-monorepo#1534 after the v5.5.1 release, if this passes (so the Hardhat E2E part) this should give additional assurance that the fix is correct.

@holgerd77
Copy link

Update: yes, Hardhat E2E tests are passing again. 😄 (might be worth to think if you also directly want to integrate these tests in your CI? Then you would be able to always test your latest master state towards the Hardhat test suite. Not sure how easily this would be adjustable though, but I could imagine it would be not too much work)

@hogan0520
Copy link
Author

Fixed! THX!

@voltrevo
Copy link

It seems ethers@5.5.1 mostly pulls in @ethersproject dependencies at version 5.5.0:

{
  "author": "Richard Moore <me@ricmoo.com>",
  "browser-old": "./dist/ethers.umd.js",
  "dependencies": {
    "@ethersproject/abi": "5.5.0",
    "@ethersproject/abstract-provider": "5.5.1",
    "@ethersproject/abstract-signer": "5.5.0",
    "@ethersproject/address": "5.5.0",
    "@ethersproject/base64": "5.5.0",
    "@ethersproject/basex": "5.5.0",
    "@ethersproject/bignumber": "5.5.0",
    "@ethersproject/bytes": "5.5.0",
    "@ethersproject/constants": "5.5.0",
    "@ethersproject/contracts": "5.5.0",
    "@ethersproject/hash": "5.5.0",
    "@ethersproject/hdnode": "5.5.0",
    "@ethersproject/json-wallets": "5.5.0",
    "@ethersproject/keccak256": "5.5.0",
    "@ethersproject/logger": "5.5.0",
    "@ethersproject/networks": "5.5.0",
    "@ethersproject/pbkdf2": "5.5.0",
    "@ethersproject/properties": "5.5.0",
    "@ethersproject/providers": "5.5.0", // <--- 😅
    "@ethersproject/random": "5.5.0",
    ...
  },
  ...
  "version": "5.5.1"
}

Is this normal?

Anyway pulling in @ethersproject/providers@5.5.0 seems to suffer from the original issue:

Check file:///home/voltrevo/workspaces/ethf/bls-wallet/aggregator/deps.ts
error: TS2416 [ERROR]: Property 'resolveName' in type 'BaseProvider' is not assignable to the same property in base type 'Provider'.
  Type '(name: string | Promise<string>) => Promise<string | null>' is not assignable to type '(name: string | Promise<string>) => Promise<string>'.
    Type 'Promise<string | null>' is not assignable to type 'Promise<string>'.
      Type 'string | null' is not assignable to type 'string'.
        Type 'null' is not assignable to type 'string'.
    resolveName(name: string | Promise<string>): Promise<null | string>;
    ~~~~~~~~~~~
    at https://cdn.esm.sh/v54/@ethersproject/providers@5.5.0/lib/base-provider.d.ts:132:5

(Seeing this in deno trying to import https://esm.sh/ethers@5.5.1.)

@ricmoo
Copy link
Member

ricmoo commented Oct 21, 2021

It seems ethers@5.5.1 mostly pulls in @ethersproject dependencies at version 5.5.0:

Yes, that's normal. The patch version does not change. Otherwise, on every small change to any package, all packages would need to be re-published, and all audits would need to be performed on code that has zero changes except for the version in the package.json.

So, the patch can drift. The major and minor will always match though. :)

@ricmoo ricmoo removed the on-deck This Enhancement or Bug is currently being worked on. label Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue.
Projects
None yet
Development

No branches or pull requests

6 participants