Skip to content

Add support for jsconfig.json in language service #6545

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 22, 2016
Merged

Add support for jsconfig.json in language service #6545

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b5ed7f3
Add support for jsconfig.json in language service
zhengbli Jan 20, 2016
d64b603
revert breaking changes
zhengbli Jan 21, 2016
ea94a05
Add support for jsconfig in shims
Jan 21, 2016
31f5502
set default module to commonjs for jsconfig.json
Jan 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,8 @@ namespace ts {
* @param basePath A root directory to resolve relative path entries in the config
* file to. e.g. outDir
*/
export function parseJsonConfigFileContent(json: any, host: ParseConfigHost, basePath: string, existingOptions: CompilerOptions = {}): ParsedCommandLine {
const { options: optionsFromJsonConfigFile, errors } = convertCompilerOptionsFromJson(json["compilerOptions"], basePath);
export function parseJsonConfigFileContent(json: any, host: ParseConfigHost, basePath: string, existingOptions: CompilerOptions = {}, configFileName?: string): ParsedCommandLine {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is going to be an API breaking change. can we add an optional argument for the filename?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

const { options: optionsFromJsonConfigFile, errors } = convertCompilerOptionsFromJson(json["compilerOptions"], basePath, configFileName);

const options = extend(existingOptions, optionsFromJsonConfigFile);
return {
Expand Down Expand Up @@ -523,7 +523,7 @@ namespace ts {
for (const extension of supportedExtensions) {
const filesInDirWithExtension = host.readDirectory(basePath, extension, exclude);
for (const fileName of filesInDirWithExtension) {
// .ts extension would read the .d.ts extension files too but since .d.ts is lower priority extension,
// .ts extension would read the .d.ts extension files too but since .d.ts is lower priority extension,
// lets pick them when its turn comes up
if (extension === ".ts" && fileExtensionIs(fileName, ".d.ts")) {
continue;
Expand All @@ -547,10 +547,14 @@ namespace ts {
}
}

export function convertCompilerOptionsFromJson(jsonOptions: any, basePath: string): { options: CompilerOptions, errors: Diagnostic[] } {
export function convertCompilerOptionsFromJson(jsonOptions: any, basePath: string, configFileName?: string): { options: CompilerOptions, errors: Diagnostic[] } {
const options: CompilerOptions = {};
const errors: Diagnostic[] = [];

if (configFileName && getBaseFileName(configFileName) === "jsconfig.json") {
options.allowJs = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default "module" to "commonjs" also for jsconfig.json files, as by default we infer CommonJS from require calls or exports assignments, and also this will allow usage of ES6 syntax without getting errors about setting the module type (which is mostly needed for emit, which usually doesn't apply in the .js case).

}

if (!jsonOptions) {
return { options, errors };
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/tsc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ namespace ts {
if (sys.watchDirectory && configFileName) {
const directory = ts.getDirectoryPath(configFileName);
directoryWatcher = sys.watchDirectory(
// When the configFileName is just "tsconfig.json", the watched directory should be
// When the configFileName is just "tsconfig.json", the watched directory should be
// the current direcotry; if there is a given "project" parameter, then the configFileName
// is an absolute file name.
directory == "" ? "." : directory,
Expand Down
42 changes: 24 additions & 18 deletions src/server/editorServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ namespace ts.server {
if (!resolution) {
const existingResolution = currentResolutionsInFile && ts.lookUp(currentResolutionsInFile, moduleName);
if (moduleResolutionIsValid(existingResolution)) {
// ok, it is safe to use existing module resolution results
// ok, it is safe to use existing module resolution results
resolution = existingResolution;
}
else {
Expand All @@ -145,8 +145,8 @@ namespace ts.server {
}

if (resolution.resolvedModule) {
// TODO: consider checking failedLookupLocations
// TODO: use lastCheckTime to track expiration for module name resolution
// TODO: consider checking failedLookupLocations
// TODO: use lastCheckTime to track expiration for module name resolution
return true;
}

Expand Down Expand Up @@ -483,7 +483,7 @@ namespace ts.server {
openFileRootsConfigured: ScriptInfo[] = [];
// a path to directory watcher map that detects added tsconfig files
directoryWatchersForTsconfig: ts.Map<FileWatcher> = {};
// count of how many projects are using the directory watcher. If the
// count of how many projects are using the directory watcher. If the
// number becomes 0 for a watcher, then we should close it.
directoryWatchersRefCount: ts.Map<number> = {};
hostConfiguration: HostConfiguration;
Expand Down Expand Up @@ -564,11 +564,11 @@ namespace ts.server {
// We check if the project file list has changed. If so, we update the project.
if (!arrayIsEqualTo(currentRootFiles && currentRootFiles.sort(), newRootFiles && newRootFiles.sort())) {
// For configured projects, the change is made outside the tsconfig file, and
// it is not likely to affect the project for other files opened by the client. We can
// it is not likely to affect the project for other files opened by the client. We can
// just update the current project.
this.updateConfiguredProject(project);

// Call updateProjectStructure to clean up inferred projects we may have
// Call updateProjectStructure to clean up inferred projects we may have
// created for the new files
this.updateProjectStructure();
}
Expand Down Expand Up @@ -792,8 +792,8 @@ namespace ts.server {
* @param info The file that has been closed or newly configured
*/
closeOpenFile(info: ScriptInfo) {
// Closing file should trigger re-reading the file content from disk. This is
// because the user may chose to discard the buffer content before saving
// Closing file should trigger re-reading the file content from disk. This is
// because the user may chose to discard the buffer content before saving
// to the disk, and the server's version of the file can be out of sync.
info.svc.reloadFromFile(info.fileName);

Expand Down Expand Up @@ -891,8 +891,8 @@ namespace ts.server {
}

/**
* This function is to update the project structure for every projects.
* It is called on the premise that all the configured projects are
* This function is to update the project structure for every projects.
* It is called on the premise that all the configured projects are
* up to date.
*/
updateProjectStructure() {
Expand Down Expand Up @@ -946,7 +946,7 @@ namespace ts.server {

if (rootFile.defaultProject && rootFile.defaultProject.isConfiguredProject()) {
// If the root file has already been added into a configured project,
// meaning the original inferred project is gone already.
// meaning the original inferred project is gone already.
if (!rootedProject.isConfiguredProject()) {
this.removeProject(rootedProject);
}
Expand Down Expand Up @@ -1026,10 +1026,16 @@ namespace ts.server {
// the newly opened file.
findConfigFile(searchPath: string): string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a findConfigFile function in program.ts also which only looks for tsconfig.json currently. This appear to only be used by the tsc command-line compiler though, so not needed for this review. Please add an issue to fix it though for the effort to compiler JavaScript at the command line (issue #4792).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened issue #6561 for further discussion

while (true) {
const fileName = ts.combinePaths(searchPath, "tsconfig.json");
if (this.host.fileExists(fileName)) {
return fileName;
const tsconfigFileName = ts.combinePaths(searchPath, "tsconfig.json");
if (this.host.fileExists(tsconfigFileName)) {
return tsconfigFileName;
}

const jsconfigFileName = ts.combinePaths(searchPath, "jsconfig.json");
if (this.host.fileExists(jsconfigFileName)) {
return jsconfigFileName;
}

const parentPath = ts.getDirectoryPath(searchPath);
if (parentPath === searchPath) {
break;
Expand All @@ -1053,9 +1059,9 @@ namespace ts.server {
}

/**
* This function tries to search for a tsconfig.json for the given file. If we found it,
* This function tries to search for a tsconfig.json for the given file. If we found it,
* we first detect if there is already a configured project created for it: if so, we re-read
* the tsconfig file content and update the project; otherwise we create a new one.
* the tsconfig file content and update the project; otherwise we create a new one.
*/
openOrUpdateConfiguredProjectForFile(fileName: string) {
const searchPath = ts.normalizePath(getDirectoryPath(fileName));
Expand Down Expand Up @@ -1180,7 +1186,7 @@ namespace ts.server {
return { succeeded: false, error: rawConfig.error };
}
else {
const parsedCommandLine = ts.parseJsonConfigFileContent(rawConfig.config, this.host, dirPath);
const parsedCommandLine = ts.parseJsonConfigFileContent(rawConfig.config, this.host, dirPath, /*existingOptions*/ {}, configFilename);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to make a similar change at https://github.com/Microsoft/TypeScript/blob/master/src/services/shims.ts#L949 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I should, Visual Studio should get the same support with this PR. Will update.

Debug.assert(!!parsedCommandLine.fileNames);

if (parsedCommandLine.errors && (parsedCommandLine.errors.length > 0)) {
Expand Down Expand Up @@ -1259,7 +1265,7 @@ namespace ts.server {
info = this.openFile(fileName, /*openedByClient*/ false);
}
else {
// if the root file was opened by client, it would belong to either
// if the root file was opened by client, it would belong to either
// openFileRoots or openFileReferenced.
if (info.isOpen) {
if (this.openFileRoots.indexOf(info) >= 0) {
Expand Down