-
Notifications
You must be signed in to change notification settings - Fork 657
Fix tsconfig null override for extended configurations #1313
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
Conversation
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
null
ing out extended tsconfig.json properties
The tests added here are bad; delete them and instead add tests to the existing tsconfig parsing tests in the tsoptions package, which generate better baselines. Also, make sure this works for other compiler options, not just "types". |
…ypes Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Addressed in commit 6d21ec0. Deleted the bad compiler tests and added comprehensive tsoptions package tests that properly test null overrides for array fields (types, lib, typeRoots), string fields (outDir, baseUrl, rootDir), and mixed field types including Tristate fields. The tests use the baseline system and work with both JSON API and JSONSourceFile API. |
…back - Use strings.Cut instead of strings.Split for better performance - Use targetField.SetZero() instead of reflect.Zero() for cleaner code - Restructure function with early continues to reduce nesting and improve readability Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
- Fixed line 1043 to pass extendedConfig.raw instead of nil to mergeCompilerOptions - Added test case demonstrating null overrides in middle level of extends chain - This ensures null values are properly handled at each level of tsconfig extension Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
This PR fixes support for "nulling out" extended tsconfig.json properties by setting them to
null
.Problem
When a tsconfig.json extends another tsconfig.json and tries to set a property to
null
to clear it, the current implementation doesn't handle this correctly. Consider:Before this fix:
After this fix:
Root Cause
The issue was in the
mergeCompilerOptions
function which usesIsZero()
to check if a field should be merged. In Go:null
in JSON → Gonil
slice →IsZero()
returnstrue
→ merge skips it[]
in JSON → Go empty slice →IsZero()
returnsfalse
→ merge applies itnil
slice →IsZero()
returnstrue
→ merge skips itThis meant the merge function couldn't distinguish between "not set" and "explicitly set to null".
Solution
mergeCompilerOptionsWithRaw
: A new merge function that's aware of explicitly set null values in the raw JSONhandleExplicitNullValues
: Processes raw JSON to detect fields explicitly set tonull
and applies them as overridesTest Coverage
Added comprehensive test cases:
tsconfigNullOverride.ts
)tsconfigNullOverrideMultipleFields.ts
)All existing tests pass, confirming backward compatibility.
Fixes #768.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.