Fix during porting tsc --composite tests #1590
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues identified during the porting of TypeScript's tsc --composite tests to the Go-based TypeScript compiler port. The changes include adding comprehensive test coverage for composite scenarios and fixing a null pointer dereference bug in the checker.
- Added extensive test cases covering composite option behavior with various command-line argument combinations
- Fixed a potential null pointer dereference in the external module resolution code
- Added test baselines covering JSX synthetic imports, module conversion scenarios, and composite option edge cases
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| testdata/baselines/reference/tsc/composite/*.js | New baseline files for composite test scenarios |
| internal/execute/tsc_test.go | Added TestTscComposite function with comprehensive test cases |
| internal/checker/checker.go | Fixed null pointer dereference by adding nil check for ImportClause |
| commandLineArgs: []string{"--composite", "false"}, | ||
| }, | ||
| { | ||
| subScenario: "when setting composite false on command line but has tsbuild info in config", |
There was a problem hiding this comment.
This test case has an identical subScenario name to the previous test case (line 173). This will cause confusion and potentially interfere with test identification. Each test case should have a unique subScenario name.
| subScenario: "when setting composite false on command line but has tsbuild info in config", | |
| subScenario: "when setting composite false on command line but has tsbuild info in config (variant 2)", |
Copilot uses AI. Check for mistakes.
| commandLineArgs: []string{"--composite", "false"}, | ||
| }, | ||
| { | ||
| // !!! sheetal null is not reflected in final options |
There was a problem hiding this comment.
The comment contains what appears to be a personal name 'sheetal' which seems out of place in a code comment. This should be removed or clarified.
| // !!! sheetal null is not reflected in final options | |
| // Setting composite to null on the command line is not reflected in final options |
Copilot uses AI. Check for mistakes.
|
This was cherry pick from #1484 but given there are more tests that were fixed as part of tsc -b work, closing this |
1 participant
No description provided.