tsdx silently fails parsing of tsconfig files containing comments, trailing commas, etc. that TSC accepts

[justingrant]

Current Behavior

tsc supports tsconfig.json files that contain comments, trailing commas, and other features beyond standard JSON.

Current tsdx, however, parses tsconfig.json using fs.readJson which ends up calling JSON.parse which will throw an exception if faced with comments, trailing commas, and other non-standard JSON content. Making things worse, the exception is silently ignored, which makes it really hard to figure out why config settings are being ignored. I had to step through tsdx code to figure out what was going on.

tsdx/src/createRollupConfig.ts

Lines 46 to 49 in 4f6de10

	let tsconfigJSON;
	try {
	tsconfigJSON = await fs.readJSON(paths.tsconfigJson);
	} catch (e) {}

Luckily this JSON is only used once in tsdx (only in createRollupConfig.ts) and that JSON is only currently used for one setting: esModuleInterop. But #468 is about to use it too for processing declarationDir.

BTW, tsdx is in good company with this problem: Create React App had the same problem.

There's also an extends feature in tsconfig which allows configuration inheritance across multiple tsconfig files. This isn't supported by tsdx either. See #484.

Expected behavior

1. tsdx should accept the same tsconfig.json syntax that does.
2. Syntax errors (or other unexpected errors) reading tsconfig.json should not be silently ignored.

Suggested solution(s)

For 1) Luckily, the nice people in TypeScript-land have exported their tsconfig parser. From facebook/create-react-app#6865 (comment):

Unfortunately it looks like typescript uses a hand-written parser. I traced it down to parseJsonText which is defined in src/compiler/parser.ts. Ahahaha.

If we're allowed to hook into the typescript package, it looks like they provide public access to their parse function.

Note that there are a whole set of parse functions defined. Hopefully one of those would meet tsdx's needs. Ideally it'd also support the extends keyword in tsconfig.json which is also currently unsupported by tsdx.

for 2) I'd suggest white-listing ignorable exceptions (e.g. file not found?) and reporting the rest to the user. In particular, any syntax errors should be reported to the user and should fail the build.

Additional context

Your environment

Software	Version(s)
TSDX	[email protected]
TypeScript	[email protected]
Browser	n/a (this is a build issue)
npm/Yarn	[email protected]
Node	v12.13.1
Operating System	MacOS Catalina 10.15.2

[agilgur5]

Awesome description and investigation @justingrant !!

I haven't read through the CRA issues/code (don't have time right now, will check later), but your reasoning and that solution according to it make sense to me. Would you like to add a PR(s) for this and #484 ?
It will merge conflict with #436 unfortunately as that has yet to be reviewed/merged by Jared 😕

---

I actually wrote that tsconfig.json parsing as my first PR to fix #165 , per that I just copied the same logic as existed for parsing package.json. This would definitely improve that naive tsconfig parsing, which is a lot more complex than package.json. The error handling is also the same as the package.json one, and that silent catch is probably problematic there too.

[justingrant]

Hi @agilgur5 - Thanks! I don't think I'll have time to PR this problem in the near future-- very busy month ahead for me and I was lucky to have time over this weekend to report some issues. Sorry I can't help more, but at least I wanted to record the problem before I got sucked into other tasks on Monday.

[agilgur5]

No worries -- figured I'd ask since you've done a lot of the investigation toward the solution already! And always good to encourage contributors in general in OSS 🙂 I'll make sure to add you to allcontributors for bugs and ideas if somebody else (probably me) PRs that

[agilgur5]

Merged in #489 as I'm now a collaborator. Turns out that ts.parseJsonConfigFileContent is needed to parse extends, but ts.readConfigFile is enough for comments and commas. These are not remotely well documented, so I had to search around other repos to figure out how to do this simply and properly. Can read the PR comments for what I searched through

[agilgur5]

@allcontributors please add @justingrant for bugs, ideas

[allcontributors]

@agilgur5

I've put up a pull request to add @justingrant! 🎉