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

tsconfig-paths behaves different from tsc and ts-node when resolving ambient declarations #51

Closed
ljani opened this issue Jun 29, 2018 · 8 comments
Labels

Comments

@ljani
Copy link
Contributor

ljani commented Jun 29, 2018

As noticed in TypeStrong/ts-node#615, tsconfig-paths behaves differently from tsc and ts-node when paths contains a directory only having index.d.ts file for a module and the module itself can be found from node_modules.

I've created a small sample here:
ljani/tsconfig-paths-51

To reproduce issue, run:

tsconfig-paths-51 >>> npm run tsc
...
{ [Function: creator] process: [Function] }

tsconfig-paths-51 >>> npm run ts-node
...
{ [Function: creator] process: [Function] }

tsconfig-paths-51 >>> npm run node
...
Error: Cannot find module 'C:\tsconfig-paths-51\types\postcss-nested'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:594:15)
    at Function.Module._resolveFilename (C:\tsconfig-paths-51\node_modules\tsconfig-paths\lib\register.js:29:44)
    at Function.Module._load (internal/modules/cjs/loader.js:520:25)
    at Module.require (internal/modules/cjs/loader.js:650:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (C:\tsconfig-paths-51\src\typescript.ts:1:1)
    at Module._compile (internal/modules/cjs/loader.js:702:30)
    at Module.m._compile (C:\tsconfig-paths-51\node_modules\ts-node\src\index.ts:439:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:713:10)
    at Object.require.extensions.(anonymous function) [as .ts] (C:\tsconfig-paths-51\node_modules\ts-node\src\index.ts:442:12)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! tsconfig-paths-51@0.1.0 node: `node --require ts-node/register --require tsconfig-paths/register src/javascript.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the tsconfig-paths-51@0.1.0 node script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\user\AppData\Roaming\npm-cache\_logs\2018-06-29T14_34_44_499Z-debug.log
@jonaskello
Copy link
Member

Thanks for taking the time to put together the repro 👍

I took a quick look and first just one observation. Currently the node script in the repro is node --require ts-node/register --require tsconfig-paths/register src/javascript.js. I think you can simplify it to directly load the typescript file,node --require ts-node/register --require tsconfig-paths/register src/typescript.ts since you have ts-node loaded. So the src/javascript.js file is probably unnecessary.

Actually it is probably possible to simplify it further sincetsconfig-paths works at run-time when the ts files have already been compiled to js. So the file typescript.ts has this code:

import postcssNested from "postcss-nested";

console.log(postcssNested);

Which will be compiled (simplified) into:

const postcssNested = require("postcss-nested");

console.log(postcssNested);

So we can probably simplify the repro even more by putting the above code in the javascript.js file and running without loading ts-node, like this: node --require tsconfig-paths/register src/javascript.js.

I'll make a fork of the repro and try this out.

@jonaskello
Copy link
Member

My fork is here. I removed the usage of ts-node from npm run node and I'm getting the same error.

So now to figure out what is going on. One possible scenario is that tsconfig-paths somehow resolves "postcss-nested" into the types/postcss-nested/index.d.ts file. So it will tell node that the code is in that folder while, but this is not true since that folder has no executable code, only type definitions. So what should happen instead is that tsconfig-paths should not resolve this, and then node will continue it's usual resolution logic and find the executable code in node_modules/postcss-nested. This is just a working theory, not sure if this is what actually happens.

@ljani
Copy link
Contributor Author

ljani commented Jun 30, 2018

No problem! Good job so far!

So it will tell node that the code is in that folder while, but this is not true since that folder has no executable code, only type definitions.

Yes, that's my theory as well. I did a little ad-hoc logging and as seen in the stacktrace, this gets called. So, I think tsconfig-paths should either try the found path and fallback calling the originalResolveFilename with the original arguments, or add more complex logic into detecting whenever the found path fulfills the request. I'm not sure what would be the performance implications of either.

@jonaskello
Copy link
Member

I added a test here to simulate the case in this issue's reproduction. This test passes however so it seems index.d.ts is not resolved in general. Something else must be going on...

jonaskello added a commit that referenced this issue Jun 30, 2018
@jonaskello
Copy link
Member

Did some more testing and found what is going on. The path will be resolved if the directory exists, even if no index.js exists. A failing test is here.

@jonaskello
Copy link
Member

So the bug is that tsconfig-paths will return true for from fileExists if it finds a folder with matching name because it just uses fs.existsSync and does not check if it is a file or folder.

@jonaskello jonaskello added the bug label Jun 30, 2018
@jonaskello
Copy link
Member

I puslished 3.4.2 where I think this is fixed now! @ljani please verify and if it works you can close this issue.

@ljani
Copy link
Contributor Author

ljani commented Jun 30, 2018

Nice work, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants