Conversation
p-chan
force-pushed
the
feature/add-shareable-tsconfig-support
branch
from
7fda278 to
2bc2c09
Compare
fbartho
left a comment
fbartho
left a comment
There was a problem hiding this comment.
No objection for this feature in concept, let’s just not pull in a new dependency for it.
Thanks!
| const tsConfigPath = lookupTSConfig(dir) | ||
| if (tsConfigPath) { | ||
| compilerOptions = JSON5.parse(fs.readFileSync(tsConfigPath, "utf8")) | ||
| compilerOptions = parseTsconfig(tsConfigPath) |
There was a problem hiding this comment.
This project already depends on TypeScript (see line 123), so it’s not helpful for us to pull in a 3rd-party package whose purpose is to parseTsconfig without a dependency on TypeScript!
Instead, I’d accept a PR that usests.parseConfig or a similar API that supports extends from the official dependency — that we’re already pulling in.
|
This PR looks like it’s a duplicate to #1313 — and that one has tests and an approach that better matches my feedback above. Not sure why that one stalled out though! So you might be able to pick up from where they started, or at least use it as a hint. |
2 participants
I used get-tsconfig to support tsconfig
extendsproperty.Fix #1283