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

Use TypeScript parser to read tsconfig.json #5532

Merged
merged 1 commit into from
Oct 22, 2018

Conversation

brunolemos
Copy link
Contributor

We have to read tsconfig.json using the ts parser otherwise it will fail, because it may contain things like comments and trailing comma.

One remaining annoyance is that when overriding the json it is removing all comments from the file.

@Timer
Copy link
Contributor

Timer commented Oct 22, 2018

One remaining annoyance is that when overriding the json it is removing all comments from the file.

This isn't too big of a deal when using source control -- you could only stage the lines you care about.

If you can figure something out, please send PR.

@Timer Timer added this to the 2.1 milestone Oct 22, 2018
@Timer Timer merged commit c6cdf42 into facebook:master Oct 22, 2018
@ianschmitz
Copy link
Contributor

@Timer, @brunolemos. Just a heads up that we're missing part of how TypeScript reads in tsconfig.json. If you take a look at my #5519 PR, you'll see how I was also using ts.parseJsonConfigFileContent() which takes care of resolving things like "extends": "./tsconfig.other.json".

Let me know if you want me to make a PR to add that step!

@ianschmitz
Copy link
Contributor

@Timer
Copy link
Contributor

Timer commented Oct 23, 2018

Sure, send a PR please.

zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Oct 29, 2018
nate770 pushed a commit to ONTW/create-react-app that referenced this pull request Oct 30, 2018
@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants