Allow JSON with comments in jsconfig.json
Is your proposal related to a problem?
This is a follow-on from #7248. Right now, we don't support JSONC (JSON with comments) in jsconfig.json files.
Describe the solution you'd like
After a discussion with @iansu, we see two paths:
- Implement the same solution as we did for TypeScript. This is easy, but would require us making TypeScript a dependency of
react-scripts. - The above, but instead of installing
typescriptas a dependency ofreact-scripts, we would install it to the user's project if they are using ajsconfig.jsonfile and don't havetypescriptinstalled.
Discussion is welcome.
If you're interested in picking this up, please let us know.
https://www.npmjs.com/package/json5 seems to be quite popular. Both .json and .json5 could be supported with their syntax (i.e. no jsonc/json5 in .json files)
@miraage, I personally feel that adding a dependency to read JSONC files would be a poorer solution than adding TS as a dependency... adding TS actually gives us some other benefits, like potentially pinning TS versions (so that they align with the linting configs, etc).
@mrmckeb, I would like to pick this up. Also, I think that moving TypeScript as a dependency to react-scripts sound like a breaking change, so I would rather go with the second option. What do you think?
Great, thanks @lianapache!
You should take a look at how verifyTypeScriptSetup.js works and either extend that, or see what can be shared here :)
@mrmckeb, Sure :+1: Thanks for the tip!
@mrmckeb we can pin TS version via react-dev-utils in that case, like cross-spawn and chalk.
Weldon guys
@mrmckeb What if we parse json file as string and remove all comments?
@esvyridov if you'd like to pick this up and give that a go, let me know.
@mrmckeb sure, I'll take this 👍
@mrmckeb I think I would try to implement the second path. The plan is:
- Check if there are comments in jsconfig
- If yes then I'll inform user that we found a comment in jsconfig and we need to install typescript to devDependencies in order to parse it.
- If not then just parse it
@esvyridov, what if we did this?
- Check if TS is installed, and parse with that - or parse without if not installed.
- If we fail, and TS was not installed, tell the user that they need to install TypeScript as a dependency if they want to support JSONC.
@mrmckeb PR #8140 is ready
I'd love to contribute to this issue. And I made a PR please check it:) https://github.com/facebook/create-react-app/pull/17148
I'm interested in working on this issue. Could you provide more context about what you're looking for? Any additional details about requirements or constraints would be helpful.