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

Symlinked node_modules directory confuses dts-bundle-generator #39

Closed
rhuitl opened this issue Jun 1, 2018 · 12 comments
Closed

Symlinked node_modules directory confuses dts-bundle-generator #39

rhuitl opened this issue Jun 1, 2018 · 12 comments
Assignees
Labels

Comments

@rhuitl
Copy link

rhuitl commented Jun 1, 2018

I have to admit, this is probably a quite uncommon use case, but still wanted to report it because maybe it can be fixed easily.

This is based on the same example as #37. Rename your node_modules directory and symlink it:

mv node_modules .node_modules_develop
ln -s .node_modules_develop node_modules

Then extract the typings. You will get a huge definitions file with a lot of inlined types. It seems like the symlink is confusing the logic that figures out how to deal with a dependency.

I've been using the symlink for a long time now as a technique to make it easy and fast to switch between the modules required for my develop and release branches. It hasn't caused any problems with NPM itself or other tools so far.

@timocov
Copy link
Owner

timocov commented Jun 1, 2018

Hi! It is interesting case. Let me take a look at it, but I'm not sure whether there is easy fix because I get filenames from TypeScript compiler and we do not know whether there were symlinks to file/folder or not.

@timocov
Copy link
Owner

timocov commented Jun 2, 2018

After looking over the issue I have the next conclusions:

  1. Indeed, as I said before, I get filenames from TypeScript compiler and I cannot find there the "original path" (path which is requested to resolve).
  2. Seems that I found a simple way to fix it - we need to just pass undefined or (path) => path as CompilerHost.realpath to the TypeScript. But I do not know what additional can happen in this case.

I'll add this "fix" under cli option (experimental actually), but let's wait @andy-ms in microsoft/TypeScript#12020 (comment) first - it is possible (I hope) that he remember what this can lead to.

@rhuitl
Copy link
Author

rhuitl commented Jun 5, 2018

Sounds like they added resolving of symlinks to support an npm link based workflow. That would break if you set that option and this is probably the more important scenario to support out of the box. Maybe you can add the realpath argument to the CLI so I can add it to my dts-bundle-generator call?

timocov added a commit that referenced this issue Jun 10, 2018
- Added experimental `--disable-symlinks-following` cli option
- Increased CLI help output max width up to 100 (instead of 80)

Fixes #39
@timocov
Copy link
Owner

timocov commented Jun 10, 2018

Sorry for long delay.

Please check #41 and confirm that it fix your problem. There I have added new CLI option to disable resolving symlinks (--disable-symlinks-following - resolving is enabled by default, so I believe it shouldn't break anything else).

I guess that with enabling this option we can get increased memory usage (because multiple files from node_modules can be one file, but treated as different files), increased working time (because in some scenarios we need to read/check/etc more files) and possibly some other unexpected errors/behavior. But I believe that it should work in common cases where you have symlinked node_modules folder or symlinked packages via npm link.

UPD: See microsoft/TypeScript#12020 (comment)

@timocov
Copy link
Owner

timocov commented Jun 18, 2018

@rhuitl
Copy link
Author

rhuitl commented Jun 21, 2018

After some testing I think this is not (completely) solved. The typings file has the standard ES typings in it when I used a symlinked node_modules directory (e.g. node_modules -> foobar):

Input source files:
  /path/foobar/typescript/lib/lib.dom.d.ts
  /path/foobar/typescript/lib/lib.es2015.d.ts
  /path/foobar/typescript/lib/lib.es5.d.ts
  /path/foobar/typescript/lib/lib.es2015.symbol.wellknown.d.ts
  /path/foobar/typescript/lib/lib.es2015.reflect.d.ts
  /path/foobar/typescript/lib/lib.es2015.proxy.d.ts
  /path/foobar/typescript/lib/lib.es2015.iterable.d.ts
  /path/foobar/typescript/lib/lib.es2015.symbol.d.ts
  /path/foobar/typescript/lib/lib.es2015.promise.d.ts
  /path/foobar/typescript/lib/lib.es2015.generator.d.ts
  /path/foobar/typescript/lib/lib.es2015.collection.d.ts
  /path/foobar/typescript/lib/lib.es2015.core.d.ts
  app.d.ts

I thought it was fixed because it magically works when you use ".node_modules" instead of "foobar" or ".node_modules_develop".

@timocov
Copy link
Owner

timocov commented Jun 21, 2018

Try to add symlinked folder ("foobar" or ".node_modules_develop") to exclude list in tsconfig.

@rhuitl
Copy link
Author

rhuitl commented Jun 21, 2018

No, that doesn't change anything. I also tried setting the files list to make sure tsc doesn't pick up random files.

@timocov
Copy link
Owner

timocov commented Jun 22, 2018

Could you please provide new steps to reproduce to be sure that we talk about the same things?

@timocov timocov reopened this Jun 22, 2018
@timocov timocov self-assigned this Jun 22, 2018
@rhuitl
Copy link
Author

rhuitl commented Jun 26, 2018

Yes, of course. Create a new directory. Place these files:

app.ts

export function test(): void { }

package.json

{
  "name": "dts-bundle-generator-bugreport",
  "version": "1.0.0",
  "private": true,
  "scripts": {
    "build": "tsc",
    "typings": "dts-bundle-generator --verbose --disable-symlinks-following -o app.d.ts app.ts"
  },
  "dependencies": {},
  "devDependencies": {
    "dts-bundle-generator": "^1.5.0",
    "typescript": "^2.9.1"
  }
}

tsconfig.json

{
  "compilerOptions": {
    "target": "es5",
    "declaration": true,
    "lib": ["es2015", "dom"]
  },
  "files": ["app.ts"]
}

Run npm install
Run npm run typings, look at console output and the generated typings file which look like they should.

Move the node_modules directory and symlink it:

mv node_modules foobar
ln -s foobar node_modules

Run npm run typings again, this time the console has a lot of output and the typings file contains typings from es2015 and dom.

@timocov
Copy link
Owner

timocov commented Jun 26, 2018

Thank you. I found the problem.

TypeScript compiler does not use CompilerHost.realpath to get the path to the lib files. Instead, it uses CompilerHost.getDefaultLibFileName and CompilerHost.getDefaultLibLocation.

Currently, in dts-bundle-generator I use regex to detect that some file is TypeScript's lib file (see here). It is not entirely correct, but it seems that compiler does not provide another way to detect it.

I found in TypeScript's code function Program.isSourceFileDefaultLibrary which does exactly I need to detect lib files, but it is not public for now.

I have created a feature request to move this function to the public API (see microsoft/TypeScript/issues/25225). After approving the request, I will be able to change the code to use this function and the issue will be fixed.

AFAIK this function was added in 2.6.x TypeScript's version, so we need to release the version with dropping support < 2.6.x versions.

@timocov
Copy link
Owner

timocov commented Jun 29, 2018

Version 1.6.0 is published.

@timocov timocov added Bug and removed In Progress labels Aug 13, 2018
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