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

feat: no longer use tsconfck #892

Merged
merged 6 commits into from
Jan 9, 2024
Merged

feat: no longer use tsconfck #892

merged 6 commits into from
Jan 9, 2024

Conversation

mdonnalley
Copy link
Contributor

@mdonnalley mdonnalley commented Dec 1, 2023

Drop use of tsconfck library for resolving extended tsconfigs.

Fixes #878
Fixes #907
@W-14526758@

@iowillhoit iowillhoit self-requested a review January 8, 2024 17:59
src/flags.ts Outdated Show resolved Hide resolved
@iowillhoit
Copy link
Contributor

QA Looks good. Nice work on the integration tests, I cannot think of another case that we would need to cover.

Sidenote: Do we want to account for the fact that references are excluded from inheritance (docs)? It looks like this is how it has been working for a while now (before this PR)

const result = await parse(path)
const tsNodeOpts = mergeNestedObjects(result.extended ?? [result], 'tsconfig.ts-node')
return {...result.tsconfig, 'ts-node': tsNodeOpts}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think anyone in the oclifverse would be using this util? Removing it would be a breaking change. You could leave the export and have it call readTSConfig from src/util/read-tsconfig.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was never export from src/index.ts so we're able to remove it without it being a breaking change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. They could grab the export from this file but what we export from src/index is our "contract".

@mdonnalley
Copy link
Contributor Author

Sidenote: Do we want to account for the fact that references are excluded from inheritance (docs)? It looks like this is how it has been working for a while now (before this PR)

I would wait until someone needs this before taking the time. I generally like to avoid adding support for something that there's not an immediate use case for

@iowillhoit iowillhoit merged commit 1542bd7 into main Jan 9, 2024
59 checks passed
@iowillhoit iowillhoit deleted the mdonnalley/no-tsconfck branch January 9, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsconfck module cannot be found Production Dependency Includes Typescript
2 participants