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

[BUG] tsconfig.json "extends" field not parsed by transpiler #1283

Open
zacharyliu opened this issue Jun 8, 2022 · 4 comments · May be fixed by #1397
Open

[BUG] tsconfig.json "extends" field not parsed by transpiler #1283

zacharyliu opened this issue Jun 8, 2022 · 4 comments · May be fixed by #1397
Labels
bug enhancement help wanted You Can Do This This idea is well spec'd and ready for a PR

Comments

@zacharyliu
Copy link

zacharyliu commented Jun 8, 2022

Describe the bug
When using the built-in native TypeScript transpiler, the extends field is not supported and only the explicit compilerOptions are used. This field is sometimes used in monorepos to extend a shared config, or to extend one of the official "base" templates.

To Reproduce
Steps to reproduce the behavior:
0. Install a base config: npm install @tsconfig/node16

  1. Create a tsconfig.json extending that config:
{
  "extends": "@tsconfig/node16/tsconfig.json"
}
  1. Create a Dangerfile which tries to use non-commonjs features:
import { exec } from "child_process";
  1. Try to run Danger with danger ci. It will report an error like "cannot use import outside a module", as the module: "commonjs" option from the base config does not get inherited correctly.

Expected behavior
Danger should support all official methods of configuring tsconfig.json.

software version
danger.js 11.0.7
node v16.15.0
npm 8.5.5
Operating System macOS 12.4

Additional context
I believe the bug is because Danger's transpiler directly loads the tsconfig file contents and passes it to ts.transpileModule:

if (tsConfigPath) {
compilerOptions = JSON5.parse(fs.readFileSync(tsConfigPath, "utf8"))
} else {
compilerOptions = ts.getDefaultCompilerOptions()
}
let result = ts.transpileModule(content, sanitizeTSConfig(compilerOptions))

This can be fixed by using the TypeScript APIs for parsing the config options. For example, in ts-node:
https://github.com/TypeStrong/ts-node/blob/14323f9d00d5c7051ac09b944c7f423e442145ea/src/configuration.ts#L301-L317

Some of the other functions in this file, like lookupTSConfig, can also use the native APIs instead.

@zacharyliu zacharyliu added the bug label Jun 8, 2022
@fbartho fbartho added enhancement help wanted You Can Do This This idea is well spec'd and ready for a PR labels Jun 8, 2022
@fbartho
Copy link
Member

fbartho commented Jun 8, 2022

Thanks for filing @zacharyliu -- this seems pretty important! Do you feel comfortable filing a PR for this?

@orta -- is there any reason (compatibility, peril, …) @zacharyliu's proposed change of using lookupTSConfig and parseJsonConfigFileContent wouldn't work?

https://github.com/microsoft/TypeScript/blob/b8f648832379005fc8c3c9b34cc5e4acd01653e4/src/compiler/commandLineParser.ts#L2679-L2688

@orta
Copy link
Member

orta commented Jun 8, 2022

Nope, that change should be fine I think 👍🏻

@zacharyliu
Copy link
Author

Yep, I got it working locally with a patch, so I'll try to clean it up for a PR.

@orta Do you happen to know which TypeScript APIs are best to use for this? ts-node uses ts.readConfigFile and ts.parseJsonConfigFileContent, but there's also a few other similar APIs like ts.getParsedCommandLineOfConfigFile.

@orta
Copy link
Member

orta commented Jun 8, 2022

Off-hand, no, I'd follow ts-node - given the amount of traffic they've seen, it should be a reasonable pattern to replicate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement help wanted You Can Do This This idea is well spec'd and ready for a PR
Projects
None yet
3 participants