Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Conversation

@Velenir
Copy link
Contributor

@Velenir Velenir commented Jun 16, 2020

Description

Currently if I want to use web3-eth-abi separate from the whole web3 in Typescript I get wrong types on import
Declarations in index.d.ts specify export class AbiCoder as the only export, so TS expects the following to work

import { AbiCoder } from 'web3-eth-abi'
const  coder = new AbiCoder()
coder.encodeFunctionSignature('myMethod(uint256,string)')

But in reality src/index.js exports a singleton instance of AbiCoder as default

var coder = new ABICoder();

module.exports = coder;

And TS code breaks with Uncaught TypeError: web3_eth_abi__WEBPACK_IMPORTED_MODULE_1__.AbiCoder is not a constructor
To properly use abi I have to

import Web3Abi, { AbiCoder } from 'web3-eth-abi'

const Abi = (Web3Abi as unknown) as AbiCoder

To sum it up, TS declarations export named export class, whereas real JS export is default of that class instance.
I think there's a similar problem (sans singleton aspect) in web3-eth-ens and web3-eth-contract, maybe more. Didn't look too deep into it.

This PR switches named export of class AbiCoder to type only export (as JS doesn't export class at all), and adds a default export of AbiCoder instance.
As an alternative, you may want to export both class and singleton instance in both JS and TS.

I believe #3198 is a related issue.

Type of change

Type fix

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success.
  • I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • I ran npm run build-all and tested the resulting files from dist folder in a browser.
  • I have updated the CHANGELOG.md file in the root folder.
  • I have tested my code on the live network.
  • I have checked the Deploy Preview and it looks correct.

@coveralls
Copy link

coveralls commented Jun 16, 2020

Coverage Status

Coverage decreased (-9.4%) to 80.409% when pulling 97e33b6 on Velenir:fix_abi_export_types into 8ff9d6d on ethereum:1.x.

@ryanio ryanio added 1.x 1.0 related issues Types Incorrect or missing types labels Jun 16, 2020
@cgewecke
Copy link
Collaborator

cgewecke commented Jun 26, 2020

@Velenir Apologies for the delay reviewing here...

It looks like this issue affects ~16 Web3 sub-modules and various PR's have been opened in piecemeal fashion to address each one as users have run into the bug.

One complication with fixing it is that the solution should (ideally) be compatible with near-term plans to rewrite Web3 1.x in TS. Because the types are wrong for constructor usage but people are already using them as definitions, doing this correctly may require a breaking change.

TLDR; I think we need to fix this in a consistent way across the project and have a plan for how to manage its implications.

Related issues include

cc: @GregTheGreek

@GregTheGreek
Copy link
Contributor

GregTheGreek commented Jun 29, 2020

FYI We've opened up #3606 to try and track these changes in a more consistent manner. this PR isn't necessarily off the table, I just think we need to asses everything first

@github-actions
Copy link

This PR has been automatically marked as stale beacause it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Aug 17, 2020
@github-actions github-actions bot closed this Sep 1, 2020
@GregTheGreek GregTheGreek added the work-in-progress Prevents stalebot from closing a pr label Sep 1, 2020
@GregTheGreek GregTheGreek reopened this Sep 1, 2020
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@DrZainAzm
Copy link

“Tools”,“Interact”,”Contracts”
Contract Address: https://etherscan.io/address/0x0000000000000000000000000000000000000000
ABI: http://api.etherscan.io/api?module=contract&action=getabi&address=0x0000000000000000000000000000000000000000&format=raw
Spender address (migration contract): 0x3c75d37b579e0e4896f02c0122baa4de05383a6a
“value:uint256” 8412000000000000000000
“Write”,“Generate transaction”,“Send transaction”

https://etherscan.io/token/0xed5a231ecc0fa775980ad7b86e49872feacd4c8f?a=0x3c5459bcde2d5c1edc4cc6c6547d6cb360ce5ae9

From":"0x0000000000000000000000000000000000000000","contractAddress":"0xed5a231ecc0fa775980ad7b86e49872feacd4c8f","to":"0xc287cf1f50d5bd56de70bdec1f4103d2b733a6d4","value":"8412000000000000000000","tokenName":"LuckyBucks","tokenSymbol":"LBT","tokenDecimal":"18"

Malaysia needs a privacy law that protects people, not corporations.

@privacy rights act 1974 and Securities Law act 1933 are violated by the Lucky Bucks, LLC., 5820 Live Oak Pkwy Ste 300, Norcross, 30071 Georgia, United States, Phone: +1 (770) 449-4699, E-mail: Inquiries@luckybucksga.com, Website: https://luckybucksga.com (Subsidiary).

Head Office address: Seven Aces Limited, 79 Wellington Street West, Suite 1630, PO Box 138, Toronto, Ontario, Canada M5K 1H1, Phone: +1 416.569-3292, Fax: +1 416.477-3401, Website: http://www.sevenaces.com, E-mail: Info@sevenaces.com, Email: Manish@sevenaces.com

@github-actions github-actions bot removed the Stale Has not received enough activity label Jan 23, 2021
@spacesailor24 spacesailor24 added the P3 Low severity bugs label Mar 9, 2021
@jdevcs jdevcs mentioned this pull request Apr 13, 2022
14 tasks
Copy link
Contributor

@jdevcs jdevcs left a comment

Choose a reason for hiding this comment

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

Closing this PR as there is no activity from long time. Recently merged similar fix PR #4937.

@jdevcs jdevcs closed this Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

1.x 1.0 related issues P3 Low severity bugs Types Incorrect or missing types work-in-progress Prevents stalebot from closing a pr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants