Skip to content

TypeScript Configuration Improvements - #1293

Open
gugu8intel-i9 wants to merge 1 commit into
CodebuffAI:mainfrom
gugu8intel-i9:patch-1
Open

TypeScript Configuration Improvements#1293
gugu8intel-i9 wants to merge 1 commit into
CodebuffAI:mainfrom
gugu8intel-i9:patch-1

Conversation

@gugu8intel-i9

Copy link
Copy Markdown

I improved it. I removed the browser-specific DOM library and switched module handling to NodeNext. idk what else to say, it's just a json file.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codebuff-team

Copy link
Copy Markdown
Contributor

Thanks for the PR, but this touches a shared tsconfig.base.json that is almost certainly extended by many packages (including ones with browser code and ones with non-src/dist layouts), and the change set is a wholesale rewrite rather than a fix:

  • Removing DOM from lib will break any package that compiles browser-facing code and relies on this base config for its lib list.
  • Switching from module: esnext / moduleResolution: bundler to NodeNext/NodeNext is a major semantic change - it enforces Node's ESM resolution rules (explicit file extensions, no bare directory imports, etc.) across every consumer of this base config. That's a repo-wide migration, not a config tweak, and it directly conflicts with the allowImportingTsExtensions option you removed (which was there to support the bundler resolution mode).
  • Adding rootDir: "src", outDir: "dist", and include: ["src/**/*.ts"] to the base config assumes every package uses that exact layout. Any package that doesn't (or that overrides noEmit because it's bundler-only) will now get broken output paths or files silently excluded from compilation.
  • exactOptionalPropertyTypes and noUncheckedIndexedAccess are good strictness options in isolation, but flipping them on globally will produce a large wave of new type errors across the codebase with no corresponding fixes in this PR.

The PR description itself ('idk what else to say, it's just a json file') suggests this wasn't tested against the actual packages that consume this config. A change like this needs to be scoped per-package, tested against a full build, and justified with the specific problem it solves rather than applied as a blanket 'improvement'. As-is, this would break the build for consumers of this file, so I'm closing it.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:rejected Not a change this project wants; closed with an explanation labels Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:rejected Not a change this project wants; closed with an explanation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants