chore: remove comments in tsconfig.json

[bdb2381]

JSON doesn't support comments. This PR removes the comments.

One downstream impact of these comments is Snyk fails to parse the JSON file. https://jsoneditoronline.org/indepth/parse/json-comments/ for some details

[Shinigami92]

Please note that tsconfig.json is officially written in jsonc

• Allow comments in tsconfig.json microsoft/TypeScript#4987
• Suggestion: tsconfig.*.json should be detected as jsonc microsoft/vscode#50974

So comments are officially allowed, based on what TypeScript allows

---

Theoretically we could remove these values anyway, but it might be that we would write other comments into this file anyway in a later phase for any reason

So IMO the problem is not on our side but on Snyk as they should use a jsonc / json5 parser to correctly read these kind of files

[codecov]

Codecov Report

Merging #1811 (eddff87) into next (c46c539) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head eddff87 differs from pull request most recent head 54462fa. Consider uploading reports for the commit 54462fa to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             next    #1811   +/-   ##
=======================================
  Coverage   99.63%   99.64%           
=======================================
  Files        2346     2347    +1     
  Lines      235735   235755   +20     
  Branches     1141     1230   +89     
=======================================
+ Hits       234875   234908   +33     
+ Misses        838      825   -13     
  Partials       22       22           

Impacted Files	Coverage Δ
src/modules/location/index.ts	98.70% <0.00%> (-0.22%)	⬇️
src/modules/string/index.ts	100.00% <0.00%> (ø)
src/locales/nl/person/index.ts	100.00% <0.00%> (ø)
src/locales/vi/person/male_first_name.ts	100.00% <0.00%> (ø)
src/locales/vi/person/female_first_name.ts	100.00% <0.00%> (ø)
src/locales/nl/person/tussenvoegsel.ts	100.00% <0.00%> (ø)
src/modules/internet/user-agent.ts	96.74% <0.00%> (+4.43%)	⬆️

[bdb2381]

That is new learning, jsonc. Thank you for the docs. I'll review so I have a better understanding.

Agreed it is handy to have comments. And agree it is something Snyk should handle as Faker isn't the only thing Snyk fails to parse. I'll work with an AE at Snyk for it.

[xDivisionByZerox]

Theoretically we could remove these values anyway, but it might be that we would write other comments into this file anyway in a later phase for any reason

I suggest explicitly setting or removing them. Even if comments are technically allowed, the one in question do not serve any purpose currently.

[ST-DDT]

I agree with @xDivisionByZerox 's reasoning here.
These should either be activated or removed (or should have a comment why they are there).

Since @Shinigami92 added them in #651 I would like to hear his original intention with those lines and suggestions whether to enable or remove them.

[Shinigami92]

Since @Shinigami92 added them in #651 I would like to hear his original intention with those lines and suggestions whether to enable or remove them.

It's exactly what the PR title is intended to say: partially activate strict mode
At the end it would be very nice to have our project fully strict (this is a ts flag that can be set to true)
But there were some issues and they can partially get solved one by one, one after another...
And at the end, erase all the partial flags and just set strict: true

[ST-DDT]

At the end it would be very nice to have our project fully strict (this is a ts flag that can be set to true)
But there were some issues and they can partially get solved one by one, one after another...
And at the end, erase all the partial flags and just set strict: true

I created an issue for each of them:

• Turn on strictNullChecks in tsconfig #1834
• Turn on strictBindCallApply in tsconfig #1835
• Turn on strictPropertyInitialization in tsconfig #1836
• Turn on noImplicitAny in tsconfig #1837
• Turn on noImplicitThis in tsconfig #1838

IMO this PR is now obsolete, since we want to actually turn on those lines.
What do you think?

[xDivisionByZerox]

IMO we can still merge this as an appreciation towards @bdb2381 for pointing out those settings.

[ST-DDT]

Since @Shinigami92 already started working on them I think this will only cause merge conflicts.

I will close this, but thanks for bringing this to our attention.

[Shinigami92]

On the one side, we could merge it, on the other side, it is more or less irrelevant as we wont release something just for this right now and therefore it would take month(s) anyway to land in "prod"