Allow JSON with comments in jsconfig.json

[mrmckeb]

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:

1. Implement the same solution as we did for TypeScript. This is easy, but would require us making TypeScript a dependency of react-scripts.
2. The above, but instead of installing typescript as a dependency of react-scripts, we would install it to the user's project if they are using a jsconfig.json file and don't have typescript installed.

Discussion is welcome.

If you're interested in picking this up, please let us know.

[miraage]

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)

[mrmckeb]

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

[lianapache]

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

[mrmckeb]

Great, thanks @lianapache!

You should take a look at how verifyTypeScriptSetup.js works and either extend that, or see what can be shared here :)

[lianapache]

@mrmckeb, Sure 👍 Thanks for the tip!

[miraage]

@mrmckeb we can pin TS version via react-dev-utils in that case, like cross-spawn and chalk.

[pardyalbert]

Weldon guys

[esvyridov]

@mrmckeb What if we parse json file as string and remove all comments?

[mrmckeb]

@esvyridov if you'd like to pick this up and give that a go, let me know.

[esvyridov]

@mrmckeb sure, I'll take this 👍

[esvyridov]

@mrmckeb I think I would try to implement the second path. The plan is:

1. Check if there are comments in jsconfig
2. 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.
3. If not then just parse it

[mrmckeb]

@esvyridov, what if we did this?

1. Check if TS is installed, and parse with that - or parse without if not installed.
2. 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.

[esvyridov]

@mrmckeb PR #8140 is ready