-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Set default types array to []; support "*" wildcard
#63054
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
base: main
Are you sure you want to change the base?
Conversation
|
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
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.
Pull request overview
This pull request implements a breaking change to TypeScript's types compiler option, setting its default to an empty array ([]) instead of automatically including all packages from node_modules/@types. A wildcard ("*") is introduced to allow users to opt into the old "include all" behavior for backward compatibility.
Changes:
- Modified
getAutomaticTypeDirectiveNamesto return[]by default whentypesis undefined and support"*"wildcard expansion - Added
usesWildcardTypeshelper function to centralize wildcard detection logic - Updated error messages to suggest adding types to the
typesfield in tsconfig when wildcard is not present - Updated 29+ tests to explicitly use
"types": ["*"]where automatic type discovery was relied upon
Reviewed changes
Copilot reviewed 171 out of 265 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/compiler/moduleNameResolver.ts | Core logic: changes getAutomaticTypeDirectiveNames to default to [] and support wildcard expansion |
| src/compiler/utilities.ts | Adds usesWildcardTypes helper to check if types array includes "*" |
| src/compiler/checker.ts | Updates error messages to conditionally suggest adding types to tsconfig |
| src/compiler/programDiagnostics.ts | Updates file include diagnostics to handle wildcard |
| src/compiler/watch.ts | Updates watch mode file include reasons for wildcard |
| src/jsTyping/jsTyping.ts | Updates type discovery to use wildcard check |
| tests/cases/fourslash/server/* | Adds "types": ["*"] to tests requiring auto-discovery |
| tests/cases/conformance/* | Adds // @types: * directive to compiler tests |
| tests/baselines/reference/* | Updates baselines for new error messages and behavior |
jakebailey
left a comment
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.
Seems like some tests are still left.
I do also wonder if we need to be forcing this on for inferred projects in tsserver...
...rence/tsbuild/moduleResolution/impliedNodeFormat-differs-between-projects-for-shared-file.js
Show resolved
Hide resolved
...rence-resolution-uses-correct-options-for-different-resolution-options-referenced-project.js
Outdated
Show resolved
Hide resolved
.../incremental/react-jsx-emit-mode-with-no-backing-types-found-doesn't-crash-under---strict.js
Outdated
Show resolved
Hide resolved
...-something-in-node_modules-or-@types-when-there-is-no-notification-from-fs-for-index-file.js
Outdated
Show resolved
Hide resolved
| [[90mHH:MM:SS AM[0m] File change detected. Starting incremental compilation... | ||
|
|
||
| [[90mHH:MM:SS AM[0m] Found 0 errors. Watching for file changes. | ||
| [96mfoo.ts[0m:[93m1[0m:[93m21[0m - [91merror[0m[90m TS2307: [0mCannot find module 'fs' or its corresponding type declarations. |
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 another case
| { | ||
| "cachedTypingPaths": [], | ||
| "newTypingNames": [], | ||
| "filesToWatch": [ |
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 wonder if we need to make services' types array default to a wildcard...
|
@typescript-bot test top800 |
|
@jakebailey Here are the results of running the top 800 repos with tsc comparing Something interesting changed - please have a look. Details
|
|
@jakebailey Here are some more interesting changes from running the top 800 repos suite Details
|
|
@jakebailey Here are some more interesting changes from running the top 800 repos suite Details
|
|
@jakebailey Here are some more interesting changes from running the top 800 repos suite Details
|
|
@jakebailey Here are some more interesting changes from running the top 800 repos suite Details
|
|
@jakebailey Here are some more interesting changes from running the top 800 repos suite Details
|
| undefined; | ||
| case FileIncludeKind.AutomaticTypeDirectiveFile: | ||
| if (!options.types) return undefined; | ||
| if (usesWildcardTypes(options)) return undefined; |
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.
when matching wildcard match to point to "wildCard" or types array?
...rence-resolution-uses-correct-options-for-different-resolution-options-referenced-project.js
Show resolved
Hide resolved
...s/reference/tscWatch/resolutionCache/works-when-included-file-with-ambient-module-changes.js
Show resolved
Hide resolved
| FileWatcher:: Close:: WatchInfo: /user/username/projects/myproject/package.json 2000 undefined File location affecting resolution | ||
| FileWatcher:: Close:: WatchInfo: /user/username/projects/package.json 2000 undefined File location affecting resolution | ||
| FileWatcher:: Close:: WatchInfo: /user/username/projects/myproject/node_modules/@types/node/ts3.6/package.json 2000 undefined File location affecting resolution | ||
| [96mworker.ts[0m:[93m1[0m:[93m1[0m - [91merror[0m[90m TS2580: [0mCannot find name 'process'. Do you need to install type definitions for node? Try `npm i --save-dev @types/node`. |
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.
Similarly here; the test was modified to add node types, I think
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.
Yeah, this test wants *
| "compilerOptions": { | ||
| "strict": true | ||
| } | ||
| }, |
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.
is this incorrect change to test - types should be inside compilerOptions ? - thats why pg is no more included in the program?
| case FileIncludeKind.AutomaticTypeDirectiveFile: | ||
| if (!options.types) return undefined; | ||
| if (usesWildcardTypes(options)) return undefined; | ||
| configFileNode = getOptionsSyntaxByArrayElementValue(getCompilerOptionsObjectLiteralSyntax(), "types", reason.typeReference); |
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.
| configFileNode = getOptionsSyntaxByArrayElementValue(getCompilerOptionsObjectLiteralSyntax(), "types", reason.typeReference); | |
| configFileNode = getOptionsSyntaxByArrayElementValue(getCompilerOptionsObjectLiteralSyntax(), "types", usesWildcardTypes(options) ? "*" : reason.typeReference); | |
|
|
||
| //// [/users/username/projects/project/foo.js] file written with same contents | ||
| Timeout callback:: count: 1 | ||
| 8: timerToUpdateProgram *new* |
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 think you want to add one more runTimeoutcallback call to test to show that it updates with no error
| 7: timerToUpdateProgram *new* | ||
| 12: timerToInvalidateFailedLookupResolutions *new* | ||
| Timeout callback:: count: 1 | ||
| 4: timerToInvalidateFailedLookupResolutions *new* |
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.
same for this. as this is checking npm install works
Redo of #63031 with slightly more manageable commit history