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

[ethers-v5] Contract types not exported in generated index.ts #278

Closed
tuler opened this issue Sep 10, 2020 · 21 comments · Fixed by #286
Closed

[ethers-v5] Contract types not exported in generated index.ts #278

tuler opened this issue Sep 10, 2020 · 21 comments · Fixed by #286

Comments

@tuler
Copy link
Contributor

tuler commented Sep 10, 2020

The index.ts only reexports the factory types, it does not export the contract types, the ones at <ContractName>.d.ts.
These types should also be exported.

@zemse
Copy link
Contributor

zemse commented Sep 11, 2020

I did try this in my PR but faced some typescript errors.

I had two ways of doing this:

1. Export both definations and factories from index.ts

Issue: Typescript doesn't complain at all. ts-node throws an error (in my project which consumes typechain and runs mocha tests with ts-node) while requiring these files:

/project-path/node_modules/ts-node/src/index.ts:586
          throw new TypeError(
                ^
TypeError: Unable to require file: build/typechain/ERC20.d.ts
This is usually the result of a faulty configuration or import. Make sure there is a `.js`, `.json` or other executable extension with loader attached before `ts-node` available.

2. An attempt to fix faulty export I tried exporting definitions from index.d.ts and factories from index.ts

Issue: Now typescript complains because it tries to look in index.ts and ignores index.d.ts for the contract types.

So I gave up for this one while submitting PR #250 thinking to look at later. @krzkaczor, anything we can do to do this or could this be a bug in ts-node? Since typescript doesn't error for the first case, I think this can be done someway.

Edit: I can recall that I tried the solution mentioned in TypeStrong/ts-node#797, but it doesn't solve the problem.

@tuler
Copy link
Contributor Author

tuler commented Sep 11, 2020

@zemse do you still have a branch with that try hanging around?
How did you try the re-export of option 1?

It should be:

export * from "./<ContractName>";

not

export * from "./<ContractName>.d.ts";

@zemse
Copy link
Contributor

zemse commented Sep 11, 2020

I was using export * from "./<ContractName>.d";. Pushed it here.

I tried with export * from "./<ContractName>";, it errors Error: Cannot find module './<ContractName>'

@zemse
Copy link
Contributor

zemse commented Sep 11, 2020

export * from "./<ContractName>";

Does this line in your index.ts file work in your project repo?

@tuler
Copy link
Contributor Author

tuler commented Sep 11, 2020

This change works for me.

-  .map((fileName) => [`export * from './${fileName}Factory'`, `export * from './${fileName}.d'`].join('\n'))
+  .map((fileName) => [`export * from './${fileName}Factory'`, `export * from './${fileName}'`].join('\n'))

But there is a catch. I know it only makes sense to export factories of contracts which are concrete, excluding abstracts and interfaces.

But it makes sense to export types of interfaces. In fact when you cast a contract in your test code you should usually cast to the interface, not the concrete implementation.

@zemse
Copy link
Contributor

zemse commented Sep 11, 2020

Hi, I looked into this, still getting error. I have created a minimal reproduction repo with typescript files.

https://github.com/zemse/types-import-problem

The typescript compiler doesn't give any error. But when you run the code with node js, it gives an error for the type file.

Can you have a look?

@tuler
Copy link
Contributor Author

tuler commented Sep 14, 2020

I see the problem. tsc does not use and does not copy the type definition file (.d.ts) to the destination folder.
So why don't you rename type.d.ts to type.ts, and let tsc generate the type.d.ts?

I mean:
https://github.com/zemse/TypeChain/blob/445ecfff8f141dfe22b3dcc66bbd4dd61e723496/packages/target-ethers-v5/src/index.ts#L99

-      path: join(this.outDirAbs, `${contract.name}.d.ts`),
+      path: join(this.outDirAbs, `${contract.name}.ts`),

@zemse
Copy link
Contributor

zemse commented Sep 14, 2020

That really needs to be a type definition file because it only has method signatures and doesn't have method implementations. You can see that by renaming a generated .d.ts file into .ts.

I think it could be something unexpected happening on the typescript side (since TS compiler doesn't error) so I've created a thread on Typescript repo microsoft/TypeScript#40517.

@tuler
Copy link
Contributor Author

tuler commented Sep 14, 2020

Even though it only defines types I think it should be a .ts not a .d.ts file.
Everything in a .d.ts file can be in a .ts file.
Let the compiler generate the type definition for you, like it is doing for the factories.

@zemse
Copy link
Contributor

zemse commented Sep 14, 2020

Everything in a .d.ts file can be in a .ts file.

There are methods in it without implementations which doesn't make sense in a .ts file.

I think we would need the following structure:

<ContractName>.js // exports actual js logic `ContractFactory`
<ContractName>.d.ts // exports type definations of `Contract` and `ContractFactory`.
...
...
index.js // exports js `ContractFactory` objects
index.d.ts // exports all type definations of `Contract` and `ContractFactory

I see this way it is possible. I can give a trial on this later this month. cc: @krzkaczor

@krzkaczor
Copy link
Member

Can u help me to understand why it's needed at all? I just import directly from exact files. CC: @quezak :)

@tuler
Copy link
Contributor Author

tuler commented Sep 15, 2020

Imagine you want to create a reusable npm package with the code generated by TypeChain, instead of generating it on demand in every project you want to use it.

In that case you want a package.json main key and a typescript types key with everything you typically need in the usage of package, which includes the contract types, so you can just do import { MyContract } from "my_package";.

In the current state you have to do some trickery by copying the .d.ts files over to the tsc destination directory.

@quezak
Copy link
Contributor

quezak commented Sep 15, 2020

I agree that adding a barrel file index.d.ts would help in some cases. IMO we can do this -- it has little overhead on the generation process and is not a breaking change.

@krzkaczor
Copy link
Member

Okay, I get it now. @zemse it would be cool if you could work on this.

@zemse
Copy link
Contributor

zemse commented Sep 23, 2020

Planning to work on it this weekend

I agree that adding a barrel file index.d.ts would help in some cases. IMO we can do this -- it has little overhead on the generation process and is not a breaking change.

I did not have luck with having index.d.ts in presence of index.ts. When I try to import, the module resolver gives preference to index.ts, and seems that the index.d.ts file ends up getting ignored.

I thinking to go ahead with this solution #278 (comment). And with this pattern, I think this would be a breaking change since there would be ERC20.js and ERC20.d.ts file (example). Those importing directly from index.ts won't face the problem, but to those who imported from ERC20Factory.ts since the file would be removed (replaced into ERC20.js). Please correct me if I'm wrong.

@tuler
Copy link
Contributor Author

tuler commented Sep 24, 2020

IMHO you should generate typescript code, and let the tsc compiler generate the type definition.

@zemse
Copy link
Contributor

zemse commented Sep 27, 2020

Hi @tuler can you try this in your project #279?

@pcowgill
Copy link
Contributor

I'm using a private npm package where the TypeChain-generated code with @typechain/ethers-v5 is exposed in that package. I'm using it in a React app (specifically a Next.js app) using TypeScript with a version >4, and I'm getting an error when it hits the export type line when I import from the root of the TypeChain output, or an error at the import type line if I try to import one of the .ts factory files directly where it imports the .d.ts file.

You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders

I'm seeing if I can get Webpack to ignore the .d.ts files to get around this, but if anyone is aware of another workaround, please let me know. Thanks!

@dang1412
Copy link

I'm using a private npm package where the TypeChain-generated code with @typechain/ethers-v5 is exposed in that package. I'm using it in a React app (specifically a Next.js app) using TypeScript with a version >4, and I'm getting an error when it hits the export type line when I import from the root of the TypeChain output, or an error at the import type line if I try to import one of the .ts factory files directly where it imports the .d.ts file.

You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders

I'm seeing if I can get Webpack to ignore the .d.ts files to get around this, but if anyone is aware of another workaround, please let me know. Thanks!

I am stucking the same thing, have you found the answer?

@wiredmatt
Copy link

The main issue here is that the index.ts file generated by typechain is blank:

/* Autogenerated file. Do not edit manually. */
/* tslint:disable */
/* eslint-disable */

If you could add all the exports:

export type { Ownable } from "./Ownable";
export type { ERC20 } from "./ERC20";
export type { IERC20Metadata } from "./IERC20Metadata";
export type { IERC20 } from "./IERC20";
export type { Crowdfund } from "./Crowdfund";

It would work perfectly, as you'd only need to point your main file to be dist/index.js, which will have all this generated types. Sadly it's not working for me, I'm just getting a plain index.ts file using @typechain/ethers-v5.

@wiredmatt
Copy link

wiredmatt commented Jul 12, 2022

Got it to "work" with this:

import { writeFileSync } from 'fs'
import { glob } from 'glob'

const main = async () => {
  const files = glob
    .sync('./types/ethers-contracts/*.ts', {
      ignore: ['./types/ethers-contracts/index.ts']
    })
    .map(file => file.replace('./types/ethers-contracts/', './'))
    .map(file => file.replace('.d', ''))
    .map(file => file.replace('.ts', ''))

  const exports = files.map(fileName =>
    [`export * from '${fileName}'`].join('\n')
  )

  const content =
    '/* eslint-disable node/no-missing-import */\n' +
    exports.join('\n') +
    '\n'

  writeFileSync('./types/ethers-contracts/index.ts', content)
}

main()

The problem that I'm seeing now is that the generated .d.ts files don't have their interface typed properly (for example, I have a DAI contract that should have a mint function, but it's not here):

interface DaiInterface extends ethers.utils.Interface {
  functions: {};

  events: {};
}

How did you guys made it work properly? Maybe the ethers package version I'm using is wrong?

{
"dependencies": {
    "ethers": "^5.0.0"
  },
  "devDependencies": {
    "@typechain/ethers-v5": "^10.1.0",
    "eslint-config-base": "*",
    "glob": "^8.0.3",
    "ts-node": "^10.8.2",
    "typescript": "^4.5.5"
  }
}

Update: For anyone experiencing the same this is probably an issue with the plugin hardhat-abi-exporter, just make sure to set the prettier option to false.

{
  abiExporter: [
    {
      path: join(
        __dirname,
        '../common/src/smart-contracts/ABIS'
      ),
      runOnCompile: true,
      clear: true,
      flat: true,
      spacing: 2,
      pretty: false
    }
  ]
}

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 a pull request may close this issue.

7 participants