-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Adds 'lib' reference directives #23893
Conversation
src/compiler/parser.ts
Outdated
forEach(toArray(entryOrList), (arg: PragmaPsuedoMap["reference"]) => { | ||
if (arg.arguments["no-default-lib"]) { | ||
context.hasNoDefaultLib = true; | ||
} | ||
else if (arg.arguments.types) { | ||
typeReferenceDirectives.push({ pos: arg.arguments.types.pos, end: arg.arguments.types.end, fileName: arg.arguments.types.value }); | ||
} | ||
else if (arg.arguments.lib) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#23904 Could avoid repeated code here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want me to do this or do you want to do in your other PR once this is merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll close the other PR since it doesn't have much benefit with only 2 branches, but maybe it would have a benefit with 3 branches, or there's a better way to share the code.
>Array : ArrayConstructor | ||
>from : { <T>(iterable: Iterable<T> | ArrayLike<T>): T[]; <T, U>(iterable: Iterable<T> | ArrayLike<T>, mapfn: (v: T, k: number) => U, thisArg?: any): U[]; <T>(arrayLike: ArrayLike<T>): T[]; <T, U>(arrayLike: ArrayLike<T>, mapfn: (v: T, k: number) => U, thisArg?: any): U[]; } | ||
>from : { <T>(arrayLike: ArrayLike<T>): T[]; <T, U>(arrayLike: ArrayLike<T>, mapfn: (v: T, k: number) => U, thisArg?: any): U[]; <T>(iterable: Iterable<T> | ArrayLike<T>): T[]; <T, U>(iterable: Iterable<T> | ArrayLike<T>, mapfn: (v: T, k: number) => U, thisArg?: any): U[]; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't arrayFrom.ts
have a Set
test? Then if would break due to this change since Set
is Iterable
but not ArrayLike
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a test, but this shouldn't break because there is still an overload that supports Iterable
. The order just changed because I cleaned up the default order of libs. Previously if you had a "lib": ["es5", "es2015"]
in your tsconfig.json we would actually end up inserting them in reverse order because we were doing files.unshift
for default libs.
Will this allow fixing #16077 ? |
src/compiler/core.ts
Outdated
@@ -1749,7 +1749,7 @@ namespace ts { | |||
* Case-sensitive comparisons compare both strings one code-point at a time using the integer | |||
* value of each code-point after applying `toUpperCase` to each string. We always map both | |||
* strings to their upper-case form as some unicode characters do not properly round-trip to | |||
* lowercase (such as `ẞ` (German sharp capital s)). | |||
* lowercase (such as `ẞ` (German sharp capital s)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what caused that to change, but I'll revert.
src/compiler/commandLineParser.ts
Outdated
// Internally we add some additional lib references that we only support when used as part of a | ||
// "lib" reference directive. They are not available on the command line or in tsconfig.json. | ||
/* @internal */ | ||
export const libMap = cloneMap(commandLineLibMap) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would just add this to the map. chances are no one will ever include it.. and if they did.. it does not break anything.
src/lib/es2015.full.d.ts
Outdated
@@ -0,0 +1,14 @@ | |||
/// <reference lib="es5" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not this just be:
/// <reference lib="es2015" />
/// <reference lib="dom" />
/// <reference lib="dom.iterable" />
/// <reference lib="webworker.importscripts" />
/// <reference lib="scripthost" />
/// <reference lib="dom" /> | ||
/// <reference lib="webworker.importscripts" /> | ||
/// <reference lib="scripthost" /> | ||
/// <reference lib="dom.iterable" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. i would move this just after dom
src/lib/webworker.generated.d.ts
Outdated
@@ -1,3 +1,5 @@ | |||
/// <reference lib="webworker.importscripts" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed here, the file is autogenerated, and it already has the correct definition. we add it to lib.d.ts to partially allow the lib to be used in webworkers for simple cases..
the two files dom
and webworker
have too many conflicts in between them, and can not be loaded together in the same context anyways..
so I would just revert the changes to this file, and leave it as is.
@@ -46,57 +47,5 @@ | |||
"webworker.generated": "lib.webworker.d.ts", | |||
"es5.full": "lib.d.ts", | |||
"es2015.full": "lib.es6.d.ts" | |||
}, | |||
"sources": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove the support for sources
from jake and gulp as well.
src/compiler/commandLineParser.ts
Outdated
@@ -1,6 +1,55 @@ | |||
namespace ts { | |||
/* @internal */ | |||
export const compileOnSaveCommandLineOption: CommandLineOption = { name: "compileOnSave", type: "boolean" }; | |||
|
|||
// NOTE: The order here is important to default lib ordering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would expand on this comment and say that the order of the list here is the same order the lib files have in the generated program, and add a note to see use in createProgram
src/services/goToDefinition.ts
Outdated
@@ -89,6 +89,7 @@ namespace ts.GoToDefinition { | |||
} | |||
|
|||
export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { fileName: string, file: SourceFile } | undefined { | |||
debugger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one debugger
statement that needs to be removed
Great job with this PR! The diff won't load for me so I'll just ask here 🤔 ... Does this PR actually implement For more context see: #23339 (comment) Thanks! 👍 |
lib folder should go down by 4 MB or so. |
You could always just use an older copy of the lib, if you're one of those API consumers. We ship the latest one with the latest features for consumers, but if you have constraints that force you to, usually you can just include an older copy of the lib in your project - a bunch of our older RWC tests do that. |
If we get feedback on that we can ship an all in one file as well. The biggest issue is tool authors really, like TS-loader for instance. Most users use the built in lib with no changes. |
Will it add the library to the entire program or only from the reference point and downward? |
not sure i understand the question. the only difference is instead of on file |
@rbuckton @mhegazy this has broken our own build because Now |
Looks like we need to ignore |
|
I'll take a look. Lib refs are supposed to ignore noResolve. |
This had nothing to do with |
This change adds support for a
/// <reference lib="name" />
directive, allowing a file to explicitly include an existing built-in lib file.lib
reference directive, anyno-default-lib
reference directive in the lib file is ignored. This is so that alib
reference in an imported package won't cause your current project to lose its default lib if you haven't specified one."lib"
compiler option in tsconfig.json (e.g. uselib="es2015"
and notlib="lib.es2015.d.ts"
, etc.).In the long term this can help polyfill/shim packages like core-js or es6-shim.
NOTE: Apparently we were very close to running out of heap memory when running
gulp tests
after runninggulp clean
as gulp is holding onto too much memory. Individually each task is fine, but in concert they add up to a large amount of uncollected memory. To address this, I've added a script to run some of our builds out-of-process. This is not an issue in thejake
builds.Fixes #15732.
Supersedes #15780