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

getDefaultLibFilePath finds lib.d.ts in wrong node_module (TS 1.6.2) #4929

Closed
alexeagle opened this issue Sep 22, 2015 · 6 comments
Closed

Comments

@alexeagle
Copy link
Contributor

I'm working to upgrade Angular to TS 1.6.2.

We wrap the TS language services with a broccoli wrapper for incremental builds. Here is where we lookup the location of the standard lib.d.ts:
https://github.com/angular/angular/blob/master/tools/broccoli/broccoli-typescript.ts#L191

When I run this after upgrading to TS 1.6.2, and with a log statement, I see
defaultLibFilePath /Users/alexeagle/Projects/angular/node_modules/gulp-tslint/node_modules/tslint/lib/lib.d.ts
which is strange since there is no such file anyway

$ ls node_modules/gulp-tslint/node_modules/tslint/lib
tslint.d.ts tslint.js

It seems this implementation must be searching under node_modules for a lib/ folder? I don't know how else to explain this behavior.

@alexeagle
Copy link
Contributor Author

Made some progress here. I found that
node_modules/gulp-tslint/node_modules/tslint/lib/tslint.js contains a re-implementation of function getDefaultLibFilePath so that explains why we pick up this directory.

And I think I understand that as of TS 1.6.2 the typescript.d.ts no longer declares a "typescript" module, but adds symbols to the global "ts" namespace, so I am now running into this pollution of the namespace where it was not a problem before.

FYI this is gulp-tslint@3.3.0-beta (latest)

@alexeagle
Copy link
Contributor Author

as a workaround, I lazy-require tslint in one gulp task where we need it, rather than at the top of our gulpfile.

@vladima
Copy link
Contributor

vladima commented Sep 22, 2015

interesting. As I can see tslint adds ts to the global scope so node run.js for the file below will print

/home/vladima/src/test/node_modules/tslint/lib/lib.d.ts
// file: run.js
// dependencies: typescript, tslint
var a = require('typescript');
var b = require('tslint');
console.log(ts.getDefaultLibFilePath({}));

what is interesting is that in your example you have import ts = require('typescript'). Can you check the compiled code if name ts will still be there?

@alexeagle
Copy link
Contributor Author

My report wasn't quite complete - when I encountered this issue, it was with another local change while I debugged issues with the new node-compatible module loading. I actually had no require or import of typescript at all (in order to satisfy the type-checker). That failed at runtime but only after I got past this issue.
With any of the syntax import ts = require('typescript'), var ts = require('typescript') or import * as ts from 'typescript' this problem can't be reproduced in Angular.
I think your example is the best repro, basically just demonstrating that if the require of tslint is the latest one, it clobbers definitions in the 'ts' global namespace.
Probably WAI as far as TS is concerned, maybe tslint should do something differently.

@vladima
Copy link
Contributor

vladima commented Sep 22, 2015

Just to elaborate, TypeScript does not expose global value named ts. if I modify my example to look like this

// file: run.js
var a = require('typescript');
console.log(ts.getDefaultLibFilePath({}));

then node run.js will fail because ts is not defined.

@alexeagle
Copy link
Contributor Author

I see, then I think this is not an issue for anyone, unless they are already doing something wrong, it is just surprising to get TypeScript implementation exposed in a 'ts' variable from tslint.

Please re-open if I'm getting that wrong.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants