wevm / wagmi

Reactive primitives for Ethereum apps
https://wagmi.sh
MIT License
5.97k stars 1.05k forks source link

bug: using `@wagmi/cli` in a pnpm monorepo, TS4058 | TS2742 #1918

Closed dalechyn closed 1 year ago

dalechyn commented 1 year ago

Is there an existing issue for this?

Package Version

0.1.6

Current Behavior

I want to use the @wagmi/cli generated hooks inside a pnpm turborepo and reuse it across external projects.


Attempts:

I have two possible ways to achieve that:

  1. Export the generated.ts file in the package.json: main (i have a typescript project);
  2. Compile the generated.ts with tsc and tsconfig.json: declaration: true to generate both typings and the js file.

1. Exporting generated.ts itself:

The package compiles, but when is used in another repo, the TS2742 issue appears:

../../node_modules/.pnpm/does-not-matter-sdk@1.4.0_nqqoae3ectxrvfzrr4j442idbu/node_modules/does-not-matter/contracts-sdk/sdk/generated.ts:9179:17
Type error: The inferred type of 'useAccessControl' cannot be named without a reference to '.pnpm/@ethersproject+contracts@5.7.0/node_modules/@ethersproject/contracts'. This is likely not portable. A type annotation is necessary.

In the wagmi repository itself, I've seen the next type configuration: https://github.com/wagmi-dev/wagmi/blob/0b8523e2bb0fee172b53fbf14b9b399630aa442e/tsconfig.json#L36-L42

I bet this was also an issue for wagmi monorepo too, but type inferring has successfully been fixed by those lines, as when I delete them locally, the same TS2742 is thrown.

Maybe @ethersproject/contracts is also worth adding to the list?

2. Compiling generated.ts and exporting generated.d.ts, generated.js:

The package does not compile, throwing the next error:

sdk/generated.ts:28532:17 - error TS4058: Return type of exported function has or is using name 'EventListener' from external module "/does-not-matter/contracts/node_modules/@wagmi/core/dist/index" but cannot be named.

Honestly no idea about this one.

Expected Behavior

So, I would like to export the generated file into a dedicated NPM package, and I expect the typings to be inferred correctly in my project.

Steps To Reproduce

  1. Clone the repo https://github.com/h0tw4t3r/wagmi-cli-issue, which is based on top of create-wagmi:next-cli-foundry
  2. yarn install
  3. yarn pkg:build

Link to Minimal Reproducible Example (StackBlitz, CodeSandbox, GitHub repo etc.)

https://github.com/h0tw4t3r/wagmi-cli-issue

Anything else?

No response

dalechyn commented 1 year ago

In order to avoid this issue till resolved, I now try to copy-paste the file directly in the repository, and catch this :/

Снимок экрана 2023-02-27 в 00 58 58
roninjin10 commented 1 year ago

hey @h0tw4t3r,

I looked at this briefly and noticed a 2 things

  1. This is a yarn repo not a pnpm repo. There is also a package-lock.json in this repo too. You will need to completely clean your node modules and delete some lock files to rebuild clean
  2. Your repo is failing because of your tsconfig setup and not because of the wagmi cli

Your repo builds fine for me image

I hope this helps. Try getting rid of your tsconfig changes and building the tsconfig up from a clean config.

dalechyn commented 1 year ago

hey @h0tw4t3r,

I looked at this briefly and noticed a 2 things

  1. This is a yarn repo not a pnpm repo. There is also a package-lock.json in this repo too. You will need to completely clean your node modules and delete some lock files to rebuild clean

  2. Your repo is failing because of your tsconfig setup and not because of the wagmi cli

Your repo builds fine for me

image

I hope this helps. Try getting rid of your tsconfig changes and building the tsconfig up from a clean config.

Hey there. I am sorry, but you ran the wrong script. Run yarn pkg:build to see the issue.

roninjin10 commented 1 year ago

I did run that script and the issue is your tsconfig the script uses. Hope this helps

roninjin10 commented 1 year ago

Specifically remove all the includes, excludes, paths ect. and build that config up intentionally 1 by 1.

dalechyn commented 1 year ago

@roninjin10 I don't really think that node_modules should be removed from the exclude array. Also, the paths there are set for a reason`. See what happens when your proposed changes are applied: Снимок экрана 2023-03-03 в 01 57 45

Снимок экрана 2023-03-03 в 01 57 33

The main intention here is to build the generated.ts package to get its declaration so it can be exported as both .js and .d.ts file as packages normally do.

At this point, using @wagmi/cli forces you to put the generated file straight in the frontend repo it uses. But what if you use foundry for your contracts? So should I move the contracts into the frontend repo?

That's pretty much what I'm left with, either try to package the @wagmi/cli generated output and import it as an external package listed on NPM registry, or try to put it in the monorepo (a better variant for the rapid development we have), and import it as a workspace package.

Снимок экрана 2023-03-03 в 02 01 04 With such setup, however, by providing the abitype as a path, the errors go down from 8 to 5.

In fact wagmi does that in the monorepo too. https://github.com/wagmi-dev/wagmi/blob/2cec200134e958b3e84d8787a8ac5b005959efcd/tsconfig.json#L29

roninjin10 commented 1 year ago

You are going to have to build up this tsconfig from scratch. My advice is to be aware of what every change you are making means and be more intentional about it.

As an example, you are excluding node_modules while not including node_modules. If you reference the docs you will see that your excludes isn't doing anything here. You are changing the path for abitype to point at node modules which is not something you would ever need to do in a tsconfig. You set baseUrl even though you aren't using it in one tsconfig and not the other. You are only including one generated file and not the rest of the project. Was this intentional? There is a lot wrong with this tsconfig both contributing to your issues and unrelated to your issue

dalechyn commented 1 year ago

As an example, you are excluding node_modules while not including node_modules

"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
"exclude": ["node_modules"]

As the **/*.ts pattern is used, it would also pick up the possible d.ts files that could lie under any package's dist in the node_modules, or am I wrong? However that was redudant in tsconfig.package.json as it already was there in tsconfig.json.

You are changing the path for abitype to point at node modules which is not something you would ever need to do in a tsconfig

You're god damn right. But I left that as a separate commit to ease the reproducing process and the investigation process. As I've mentioned, before, the paths and abitype thing reduces the errors from 8 to 5, eliminating all the TS2742 typing errors. So yeah that is intentional.

You set baseUrl even though you aren't using it in one tsconfig and not the other.

Indeed it is used to set the paths. You cannot set the paths without having baseURL.

You are only including one generated file and not the rest of the project. Was this intentional?

I'm referring to my reply once again:

The main intention here is to build the generated.ts package to get its declaration so it can be exported as both .js and .d.ts file as packages normally do.

That is intentional. I want to build the generated.ts generated hooks to receive the declaration and the js file to export them as a package, later to be imported either within a monorepo or externally.


I adore your intention to help but it doesn't look like you're reading my replies carefully.

tmm commented 1 year ago

Hi there! Sorry you are running into these issues. As they are specific to your TypeScript set up and not unique to wagmi, I'm going to close this issue. I recommend searching around the TypeScript repo to see if you can find something to help out there (e.g. https://github.com/microsoft/TypeScript/issues/42873). Good luck!

dalechyn commented 1 year ago

@tmm Hey there! Please tell me, is @wagmi/cli considered to be compiled to retrieve the declarations later to be exported within a dedicated package along with the compiled javascript files?

The whole issue is in declaration: true in my tsconfig.json.

I don't consider having a contracts right in the frontend repo a good architecture choice, thus I feel that I lack of this feature. I rather have it in a monorepo way to be later exposed for external usage, or as a totaly separate repository.

For now it feels that @wagmi/cli users would be forced to break the project structure for this, but ofc this can be completely my projection of thoughts on things. Снимок экрана 2023-03-03 в 02 58 04

Yet it makes it unusable in a monorepo.

DISCLAIMER: Not wagmi, but @wagmi/cli is unusable for the monorepo.

didostap commented 1 year ago

Also struggling with the same issue and believe declaration: true could fix it

roninjin10 commented 1 year ago

@didostap can you set preserveSymlinks: true in your tsconfig and let me know if that fixes it

roninjin10 commented 1 year ago

Here is a minimal repro https://github.com/roninjin10/wagmi-minimal

dalechyn commented 1 year ago

Here is a minimal repro https://github.com/roninjin10/wagmi-minimal

I think you're missing @wagmi/cli generated hooks, as the issue is not about wagmi.

dalechyn commented 1 year ago

I managed to fix this!

In fact, I forgot to create tsconfig.json in the contracts subpackage. I was thinking that @wagmi/cli uses internal tsconfig to build the types, so for anyone migrating their repos to wagmi please don't forget to create one.

akshatmittal commented 1 year ago

@h0tw4t3r How did you manage to fix it? Seems like I have a very similar issue (#2006) also in a monorepo using yarn.

dalechyn commented 1 year ago

@h0tw4t3r How did you manage to fix it? Seems like I have a very similar issue (#2006) also in a monorepo using yarn.

Hey there. Our team has spend too much time on attempts to fix it so now we moved the Foundry project inside the pnpm monorepo.

It indeed is a better architectural choice for us, but for other projects, the problem still remains unsolved.

The question is raised now at discussions: https://github.com/wagmi-dev/wagmi/discussions/2028

akshatmittal commented 1 year ago

we moved the Foundry project inside the pnpm monorepo

I wish I could do the same, but unfortunately limited on what I need to support. I think I too will drop @wagmi/cli from the project, too bad.

roninjin10 commented 1 year ago

The issue can be fixed by doing a pr to wagmi to export the types. It will likely fix itself once wagmi upgrades to 1.0.0. Worth noting the issue is actually wagmi and not the wagmi cli.

dalechyn commented 1 year ago

The issue can be fixed by doing a pr to wagmi to export the types. It will likely fix itself once wagmi upgrades to 1.0.0. Worth noting the issue is actually wagmi and not the wagmi cli.

Certainly, it will, but it's kinda frustrating to write the same bs the tool is doing.

Is there any ETA for 1.0.0 to consider if making such PR makes any sense?

roninjin10 commented 1 year ago

When i said pr to wagmi I meant literally doing a pr to wagmi to fix the issue in wagmi not in your code.

I heard 3 months or so for 1.0.0. I don't speak for wagmi but I would expect something like this to not be prioritized in meantime but I'm sure they would accept a pr

roninjin10 commented 1 year ago

Circling back this is still an issue. Min repro here https://github.com/roninjin10/wagmi-peer-dep-pnpm-repro

github-actions[bot] commented 10 months ago

This issue has been locked since it has been closed for more than 14 days.

If you found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest wagmi version. If you have any other comments you can create a new discussion.