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

Use jsonc.parse instead of JSON.parse when parsing tsconfig.json, fixing bug where `-b` flag is not activated when tsconfig.json contains comments. #67535

Merged
merged 1 commit into from Jan 30, 2019

Conversation

Projects
None yet
2 participants
@dtinth
Copy link
Contributor

dtinth commented Jan 30, 2019

This fixes a problem where the typescript VSCode task runs tsc with -p
when it should run -b when tsconfig.json has the "references" property.

{
  "extends": "./tsconfig.app.json",
  // meow
  "references": [{ "path": "./tsconfig.lib.json" }]
}

This bug happens because while tsconfig.json file allows comment, the
parsing logic here uses vanilla JSON.parse which cannot parse comments.

This commit fixes it by using jsonc.parse instead.

Use jsonc parser to parse a config file
This fixes a problem where the `typescript` VSCode task runs `tsc` with `-p`
when it should run `-b` when `tsconfig.json` has the `"references"` property.

```js
{
  "extends": "./tsconfig.app.json",
  // meow
  "references": [{ "path": "./tsconfig.lib.json" }]
}
```

This bug happens because while `tsconfig.json` file allows comment, the
parsing logic here uses vanilla `JSON.parse` which cannot parse comments.

This commit fixes it by using `jsonc.parse` instead.
@dtinth

This comment has been minimized.

Copy link
Contributor Author

dtinth commented Jan 30, 2019

Edit: It still doesn’t work because tsc requires -b to be passed before --watch,

@mjbvz mjbvz added this to the December/January 2019 milestone Jan 30, 2019

@mjbvz mjbvz merged commit ed8af3b into Microsoft:master Jan 30, 2019

2 checks passed

VS Code #20190130.17 succeeded
Details
license/cla All CLA requirements met.
@mjbvz

This comment has been minimized.

Copy link
Contributor

mjbvz commented Jan 30, 2019

Makes sense. Thanks

I think the argument ordering was just fixed by #67148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment