-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Adding tsconfig.json mixed content (script block) support #12153
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
Conversation
b90ce31
to
da7f824
Compare
@@ -285,7 +285,7 @@ namespace ts { | |||
return resolutions; | |||
} | |||
|
|||
export function createProgram(rootNames: string[], options: CompilerOptions, host?: CompilerHost, oldProgram?: Program): Program { | |||
export function createProgram(rootNames: string[], options: CompilerOptions, host?: CompilerHost, oldProgram?: Program, fileExtensionMap?: FileExtensionMap): Program { |
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 don't think we need to push concrete knowledge about these extra extensions through all layers, this will be necessary if we'll need to be able to add files with these extensions into program via imports or tripleslash references. Since both these scenarios are not supported and files with custom extensions should always be included in the list of root files I think we can:
- revert changes in program, language service host, lsHost and services
- in editor services for configured projects if host configuration specifies extra extensions and result of cracking config files contains files with these extensions - set
allowNonTsExtensions
bit onCompilerOptions
to force adding these files into program.
this way I think we can update only project system part and keep all other layers relatively untouched.
(Note: adding this until PR #12789 is merged in so that unit tests pass)
6d2b3c5
to
5f46e48
Compare
@@ -981,7 +981,7 @@ namespace ts { | |||
includeSpecs = ["**/*"]; | |||
} | |||
|
|||
const result = matchFileNames(fileNames, includeSpecs, excludeSpecs, basePath, options, host, errors); | |||
const result = matchFileNames(fileNames, includeSpecs, excludeSpecs, basePath, options, host, errors, fileExtensionMap); |
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 it not really a map anymore. maybe extraExtensions: ExtensionInfo[]
?
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.
sounds good - will switch to extraFileExtensions: FileExtensionInfo[]
…s: FileExtensionInfo[]
@@ -670,7 +678,7 @@ namespace ts.server { | |||
// check if requested version is the same that we have reported last time |
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 make it more like
let updatedFileNames = this.updatedFileNames;
this.updatedFileNames = undefined;
/// use local updatedFileNames - this way we'll know that set of names is definitely cleared
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.
Yes - good idea
export function getSupportedExtensions(options?: CompilerOptions): string[] { | ||
return options && options.allowJs ? allSupportedExtensions : supportedTypeScriptExtensions; | ||
export function getSupportedExtensions(options?: CompilerOptions, extraFileExtensions?: FileExtensionInfo[]): string[] { | ||
let typeScriptHostExtensions: string[] = []; |
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.
minor nit to reduce allocations:
let allExtensions: string[];
let allTypeScriptExtensions: string[];
const needAllExtensions = options && options.allowJs;
if (!extraFileExtensions) {
return needAllExtensions ? allSupportedExtensions : supportedTypeScriptExtensions;
}
const extensions = (needAllExtensions ? allSupportedExtensions : supportedTypeScriptExtensions).slice(0);
for (const ext of extraExtensions) {
if (!needAllExtensions || ext.scriptKind === ScriptKind.TS) {
extensions.push(ext.fileName);
}
}
return extensions;
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.
clever - testing this change and will update
What is mixed content? @ahejlsberg had that tradition of decorating significant new work with a detailed summary. I don't think it is such a bad idea. |
Adding tsconfig.json mixed content (script block) support
@vladima: I'll work on adding tests for this now.