Skip to content
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

Add support for jsconfig.json in language service #6545

Merged
merged 4 commits into from
Jan 22, 2016

Conversation

zhengbli
Copy link
Contributor

This PR adds support for jsconfig.json in the language service. The implementation treats jsconfig.json as a tsconfig.json file with the allowJs compiler option set to true.


Edit by @DanielRosenwasser: Fixes #6561.

@@ -493,8 +493,9 @@ 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 {
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

@@ -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

@zhengbli
Copy link
Contributor Author

@mhegazy @billti any other comments?

In addition, all error messages suggesting adding a new tsconfig.json should be updated with recommending both tsconfig.json and jsconfig.json. Though those can be a separate PR after 1.8 release.

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).

@billti
Copy link
Member

billti commented Jan 21, 2016

One more change, then 👍

zhengbli added a commit that referenced this pull request Jan 22, 2016
Add support for jsconfig.json in language service
@zhengbli zhengbli merged commit 931d162 into microsoft:master Jan 22, 2016
@zhengbli zhengbli deleted the supportJsconfig branch January 22, 2016 00:35
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants