jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.28k stars 507 forks source link

Error when building a file that imports .d.ts file (no .ts file) #744

Open pcowgill opened 4 years ago

pcowgill commented 4 years ago

Current Behavior

TSDX (probably Rollup behind the scenes) struggles with building a src/index.ts file that imports generated types. Here's a repo with a reproduction: https://github.com/pcowgill/tsdx-typechain/

This line - import { Greeter } from "../typechain/Greeter"; - isn't working.

Greeter is a .d.ts file.

I might need to configure Rollup via TSDX, there might be a simple change to the tsconfig.json that can address it, or maybe something else needs to be done.

Error: Could not resolve '../typechain/Greeter' from src/index.ts

    at error (/Users/paulcowgill/Code/tsdx-typechain/node_modules/rollup/dist/shared/node-entry.js:5400:30)

    at ModuleLoader.handleResolveId (/Users/paulcowgill/Code/tsdx-typechain/node_modules/rollup/dist/shared/node-entry.js:12410:24)

    at ModuleLoader.<anonymous> (/Users/paulcowgill/Code/tsdx-typechain/node_modules/rollup/dist/shared/node-entry.js:12298:30)

    at Generator.next (<anonymous>)

    at fulfilled (/Users/paulcowgill/Code/tsdx-typechain/node_modules/rollup/dist/shared/node-entry.js:38:28)

Expected behavior

tsdx build completes and uses the .d.ts type files I imported in src/index.ts.

Suggested solution(s)

I'm not sure if I need a rollup plugin, a different set of choices in tsconfig.json, or something else. If it's a rollup plugin I need, then just some docs for TSDX would be the change to make. If it's a tsconfig.json change, then maybe the default TSDX tsconfig.json could be different to accommodate this type of usage too while still supporting the existing use cases.

Additional context

I used TypeChain to generate the types since it's an Ethereum-related project, but I don't think you need to worry about how the .d.ts types were generated to debug this issue. The type I'm trying to import is here https://github.com/pcowgill/tsdx-typechain/blob/primary/typechain/Greeter.d.ts

Your environment

Software Version(s)
TSDX 0.13.2
TypeScript 3.9.5
Browser n/a
npm/Yarn npm 6.14.4
Node 12.18.0
Operating System macOS 10.15.5

Thanks!!

agilgur5 commented 4 years ago

Does tsc not give this error? Type declaration files are ambient, as far as I know, you don't import them, you just include them in your build. That's why you can override them very easily and why they can be brittle.

You can only import TS / TSX / JS files (TSDX allows JSX also), so this error is probably because a Greeter.ts file doesn't exist.

Also note, if you're planning on exporting some of these types further, TS doesn't re-export .d.ts so TSDX does not either (c.f. #113)

agilgur5 commented 4 years ago

TSDX's default tsconfig has an include (typeRoots or some other configs work for that too) for the types directory which is where you could put it

pcowgill commented 4 years ago

@agilgur5 Thanks for the quick and thoughtful responses! I'll reply to each of your comments separately.

TSDX's default tsconfig has an include (typeRoots or some other configs work for that too) for the types directory which is where you could put it

This doesn't seem to change the result. I made these changes in this feature branch. When I first put it in the types dir, there were extra errors about rootDir not containing all source files.

rpt2: options error TS6059: File '/Users/paulcowgill/Code/tsdx-typechain/types/GreeterFactory.ts' is not under 'rootDir' '/Users/paulcowgill/Code/tsdx-typechain/src'. 'rootDir' is expected to contain all source files.
rpt2: options error TS6059: File '/Users/paulcowgill/Code/tsdx-typechain/types/GreeterFactory.ts' is not under 'rootDir' '/Users/paulcowgill/Code/tsdx-typechain/src'. 'rootDir' is expected to contain all source files.
rpt2: options error TS6059: File '/Users/paulcowgill/Code/tsdx-typechain/types/GreeterFactory.ts' is not under 'rootDir' '/Users/paulcowgill/Code/tsdx-typechain/src'. 'rootDir' is expected to contain all source files.

All 3 errors above were about the .ts file that TypeChain generated, not the .d.ts file I wanted. So I tried two methods to fix that, separately:

  1. I updated the tsconfig.json to have . as the rootDir rather than ./src

  2. I updated the tsconfig.json include to say "include": ["src", "types/*.d.ts"],

Either way, the new errors go away, but the same error from before is still there, but now referencing the types dir rather than the typechain dir which wasn't "included" in the tsconfig.json in my original attempt.

Error: Could not resolve '../types/Greeter' from src/index.ts

pcowgill commented 4 years ago

Does tsc not give this error?

Nope, it doesn't. I added a script to the package.json in this feature branch, and it runs successfully and creates these files:

dist/
  index.js
  indes.d.ts
  index.js.map
agilgur5 commented 4 years ago

@agilgur5 Thanks for the quick and thoughtful responses! I'll reply to each of your comments separately

Thanks for the detailed reproduction, makes it easier to respond that way. One nit was that you didn't put the specific error here so there's a few clicks to get to it in the README, especially on mobile.

When I first put it in the types dir, there were extra errors about rootDir not containing all source files.

Yea this is because you put a source file there too, i.e. GreeterFactory.ts, per the errors. If you only put declarations there it works fine. Changing rootDir to . causes TS to compile all TS files in ., including e.g. test files, which is why there should be a warning when you do so.

Nope, it doesn't. I added a script to the package.json in this feature branch, and it runs successfully and creates these files:

Hmm, maybe it works due to more recent changes in TS. Might need to update the resolve plugin's configuration to add .d.ts as extension, which would explain why the error is "unable to resolve"

I'll need to investigate this more. In the meantime, you can try to override this yourself with tsdx.config.js as a workaround or use patch-package. I don't quite have the time right now to give directions on that but you can see src/createRollupConfig.ts to find the appropriate line to change

pcowgill commented 4 years ago

One nit was that you didn't put the specific error here so there's a few clicks to get to it in the README, especially on mobile.

Good point. I just added the error to the description of this issue!

pcowgill commented 4 years ago

@agilgur5 tsdx builds successfully when using the import type syntax rather than just import.

https://github.com/ethereum-ts/TypeChain/issues/247#issuecomment-648139358

pcowgill commented 4 years ago

It looks like the prettier + estlint config out of the box with tsdx doesn't like that syntax, though.

Any idea how to fix this linting error when using this new approach?

Parsing error: '=' expected prettier/prettier

which refers to this line in the code:

import type { Greeter as GreeterType } from './types/Greeter';

Here's a link to the reproduction for this error.

And here are the TypeScript docs for this syntax. Thanks!

agilgur5 commented 4 years ago

It looks like the prettier + estlint config out of the box with tsdx doesn't like that syntax, though.

Any idea how to fix this linting error when using this new approach?

Ack, I'm pretty sure that's because the version of Prettier we use doesn't support TS 3.8, which introduced type-only imports. Prettier v2+ supports TS 3.8. See also #657 / #632. TS 3.8 support has some build-chain issues though: https://github.com/formik/tsdx/pull/679#issuecomment-631478298 (and a number of breaking changes per #632). Will have to see if those have improved in the past few months as it blocks a few breaking upgrades (also the fact that they're breaking means it'll take time to release them). If you're able to use pieces of TS 3.8 though then maybe those issues are only for certain features of TS 3.8 and not the whole thing.

agilgur5 commented 4 years ago

Might need to update the resolve plugin's configuration to add .d.ts as extension, which would explain why the error is "unable to resolve"

So I thought I'd need to change this line to add .d.ts: https://github.com/formium/tsdx/blob/a9434f9311a0893d32e0f3e4033a9c2fbdc5e216/src/createRollupConfig.ts#L121

But actually it doesn't even have .ts or .tsx to begin with... this might actually be upstream: https://github.com/ezolenko/rollup-plugin-typescript2/issues/85

0xpelmeni commented 2 years ago

@pcowgill what was your solution to this? I tried adding import type {xxx} from './typechain/myfile.d.ts' (d.ts omitted of course) to my index.ts and this does allow tsdx to build but npm installing my package in another project still results in an error because myfile.d.ts is not included in the build, so its an empty reference.

agilgur5 commented 2 years ago

@0xpelmeni any import type statements should be ellided/removed by TS during compilation to JS as they don't exist in JS (plain import statements are different). You can also use a triple-slash reference to the same effect.

So there should be no "empty reference" as the .d.ts file doesn't exist at all from the perspective of pure JS.

If you're trying to produce declarations for another TS project to consume, then you might run into a different issue that I mentioned above:

Also note, if you're planning on exporting some of these types further, TS doesn't re-export .d.ts so TSDX does not either (c.f. #113)

Maybe that's the problem you're having, but there are too few details in your comment to tell for sure.


Side note to other readers: nowadays I help maintain upstream rpt2 (before I just contributed to fix TSDX issues), so I'm very familiar with its codebase and even more familiar with the TS Compiler API/its behavior as well as Rollup.

Per the upstream issue in rpt2, having to use import type or triple-slash directives is more or less the correct behavior as declaration files are really not meant to be directly imported, they're just meant to be referenced, and usually are found in the ambient environment.

0xpelmeni commented 2 years ago

@agilgur5 Apologies I should've given more detail. I'm using the same typechain tool as @pcowgill . Thats why I believe I have a similar issue. I have 2 Typescript projects : Project 1 : Holds smart contracts and uses typechain to generate typescript bindings. Uses tsdx to build and then npm package. Project 2: Uses npm to install Project 1 and make use of generated typescript bindings.

Project 1 has the folder structure:

src/
  index.ts
  typechain/
    MyContract.d.ts
    factories/
        MyContractFactory.ts

MyContract.d.ts and factories/MyContractFactory.ts are generated by typechain. MyContractFactory.ts imports & uses the types declared in MyContract.d.ts

index.ts is my own file contains just export * from './typechain/factories/MyContractFactory.ts';

Currently in Project 1 tsdx build works fine and i'm able to publish the npm package and pull it in Project 2 and make use of MyContractFactory.ts but any functions in MyContractFactory.tsthat return types declared in MyContract.d.ts get returned as type any

agilgur5 commented 2 years ago

Thanks those details are helpful.

Currently in Project 1 tsdx build works fine and i'm able to publish the npm package and pull it in Project 2 and make use of MyContractFactory.ts but any functions in MyContractFactory.tsthat return types declared in MyContract.d.ts get returned as type any

Gotcha, yea I'm pretty sure that's due to #113, which is actually TS's own behavior -- so TSDX and rpt2 don't diverge on that (as of right now). You'll need to copy over any declaration files into your dist/ folder (in the right location) so that they exist after publish as well. Or have typechain output to the dist/ folder initially. Basically declaration files are already compiled artifacts so TS doesn't transform them in any way when compiling, it just references them for type-checking. So you have to manually copy those if you want your consumers to be able to use them too.

0xpelmeni commented 2 years ago

@agilgur5 Thank you for the quick reply! And for the advice, so I should copy the .d.ts files into dist/ after tsdx build or before?

agilgur5 commented 2 years ago

And for the advice, so I should copy the .d.ts files into dist/ after tsdx build or before?

Could do either, but might be easier after tsdx build in your case since you'll have subdirectories created in dist/ (vs. before you might have to create those subdirs yourself).