Repository navigation
Do not process library reference directives with noLib set. - #29022
Conversation
Sheetal Nandi (sheetalkamat)
left a comment
There was a problem hiding this comment.
You would want to add test similar to tests/cases/conformance/declarationEmit/libReferenceDeclarationEmit.ts and tests/cases/conformance/declarationEmit/libReferenceDeclarationEmitBundle.ts
|
Sheetal Nandi (@sheetalkamat) thanks for the pointer! done. |
|
Sheetal Nandi (@sheetalkamat) should |
|
Wesley Wigham (@weswigham) Good point. Though the preprocess just gets you all the directives, imports in a file so that wont change(its independent of compiler options). It would be LS managed side that consumes it. Adding Andrew Casey (@amcasey) to see if this needs to be handled somewhere in managed side. |
|
Sheetal Nandi (@sheetalkamat) I'm not sure I understand the impact on the editor. What is it the LS managed side is consuming? |
|
Andrew Casey (@amcasey) I am referring to code that (as far as I remember use to be there) creates virtual project based on the precompute results?. |
|
Sheetal Nandi (@sheetalkamat) As far as I've been able to determine, that code was phased out after dev14. |
|
Friendly ping :-)
|
|
Martin Probst (@mprobst) You still need to add test similar to |
Ron Buckton (rbuckton)
left a comment
There was a problem hiding this comment.
This looks good to me. If you can resolve Sheetal Nandi (@sheetalkamat)'s concern we can go ahead and merge.
When a user sets `noLib`, this indicates that they will supply their own list of `lib*.d.ts` files as part of input sources. In this situation, TypeScript should not try to resolve library reference directives. This avoids a problem where TypeScript loads a file that e.g. contains `/// <reference lib="es2015.symbol"/>`. Previously, TypeScript would use its builtin ts.libMap and attempt to load builtin libraries from the TypeScript installation, instead of respecting the user-supplied set of libraries.
1d31646 to
f3f5877
Compare
|
Done. Sorry, I didn't quite get that you wanted both variants, makes sense Sheetal Nandi (@sheetalkamat). |
|
The lint errors in build are unrelated to this change and travis build succeeds in two flavors. Merging in. Thank you Martin Probst (@mprobst) for your contribution. |
|
Martin Probst (@mprobst) Wesley Wigham (@weswigham) Is there any programmatic way to disable this new behavior? It broke API Extractor in microsoft/rushstack#1304.
In our case, the "list of lib*.d.ts files" includes lib.es2015.d.ts whose sole function is to pull in a bunch of other files via reference directives. This list is obtained from tsconfig.json. Thus we don't have any easy way to expand the list without manually resolving the reference directives in lib.es2015.d.ts somehow. The use case: API Extractor always loads a known/recent version of the compiler engine, but it needs to be able to analyze projects whose tsconfig.json and code is intended for an older version of the compiler. To accomplish this, we set |
|
Pete Gonzalez (@octogonz): Do you need to set Another approach, since you are loading the compiler programmatically, is to provide a custom |
I wasn't aware of that API. I'll give it a try -- if it works it's probably the best approach, since it completely avoids the need to interpret |
When a user sets
noLib, this indicates that they will supply their ownlist of
lib*.d.tsfiles as part of input sources. In this situation,TypeScript should not try to resolve library reference directives.
This avoids a problem where TypeScript loads a file that e.g. contains
/// <reference lib="es2015.symbol"/>. Previously, TypeScript would useits builtin ts.libMap and attempt to load builtin libraries from the
TypeScript installation, instead of respecting the user-supplied set of
libraries.
Fixes #29021