Skip to content

ProjectService passing incorrect object to parseConfigFile()#3047

Merged
mhegazy merged 1 commit into
microsoft:masterfrom
bryanforbes:fix-tsserver-config-parse
May 6, 2015
Merged

ProjectService passing incorrect object to parseConfigFile()#3047
mhegazy merged 1 commit into
microsoft:masterfrom
bryanforbes:fix-tsserver-config-parse

Conversation

@bryanforbes
Copy link
Copy Markdown
Contributor

The return signature of readConfigFile() changed in f8424d0 and the code using it in ProjectService was never updated to match. This lead to the language services attempting to parse an object that doesn't match what is expected and using the default compiler options instead of what is defined in tsconfig.json. Similarly, the return value of the closure in getTSConfigFileInfo() was never updated to match in both places it returns.

This contribution should be covered by SitePen's corporate CLA.

The return signature of `readConfigFile()` changed in
f8424d0 and the code using it in
`ProjectService` was never updated to match. This lead to the language
services attempting to parse an object that doesn't match what is
expected and using the default compiler options instead of what is
defined in `tsconfig.json`. Similarly, the return value of the
closure in `getTSConfigFileInfo()` was never updated to match in
both places it returns.
@msftclas
Copy link
Copy Markdown

msftclas commented May 5, 2015

Hi @bryanforbes, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

Comment thread src/services/shims.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@paulvanbrenk how was that even working?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how did that even compile?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

forwardJSONCall takes a function that returns any. so it is not going to complain regardless what we put in there :)

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 5, 2015

Thanks @bryanforbes!

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 6, 2015

@bryanforbes I am just waiting on the CLA. Otherwise looks good to me. Thanks again.

@bryanforbes
Copy link
Copy Markdown
Contributor Author

@mhegazy Do I need to sign a personal CLA, or will my employer's CLA cover me?

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 6, 2015

@bryanforbes it depends if this is a personal contribution or in the course of your work. if it is later and you believe SitePen has a different agreement, then there is nothing you need to do, and I need to check with @msftclas.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 6, 2015

@bryanforbes just checked with @msftclas and they said each contributor needs a separate CLA. Sorry for the confusion on my part, I really do not understand any of this legal stuff :D

@msftclas
Copy link
Copy Markdown

msftclas commented May 6, 2015

@bryanforbes, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@bryanforbes
Copy link
Copy Markdown
Contributor Author

@mhegazy Don't worry about it. I get confused by it too. Let me know if there's anything else I need to do.

mhegazy added a commit that referenced this pull request May 6, 2015
ProjectService passing incorrect object to parseConfigFile()
@mhegazy mhegazy merged commit 6336925 into microsoft:master May 6, 2015
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented May 6, 2015

Thanks!

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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.

6 participants