Skip to content

fix: harden TypeScript, Go, and C# documentation validation - #2820

Open
rinceyuan wants to merge 3 commits into
github:mainfrom
rinceyuan:fix/docs-typescript-validation
Open

rinceyuan wants to merge 3 commits into
github:mainfrom
rinceyuan:fix/docs-typescript-validation

Conversation

@rinceyuan

@rinceyuan rinceyuan commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problems

The TypeScript documentation validator catches compiler failures but only records diagnostics associated with extracted example files. A compiler startup failure, global diagnostic, or error confined to an imported file outside the examples can therefore result in every example being reported as valid and exit 0.

On Windows, directly executing nodejs/node_modules/.bin/tsc produces ENOENT. The unchanged validator reported 201 files passed even with a deliberate TS2322 example. Bypassing only the launcher with Node also showed a real compiler exit 2 / TS2688 incorrectly reported as success.

The Go validator writes an unquoted local replacement path into go.mod. A checkout path containing spaces is split into separate tokens and rejected by the Go module parser.

The C# validator has the same false-success behavior for build failures without an extracted .cs file diagnostic. In an isolated fixture with a valid example and a malformed referenced project, real dotnet build returned MSB4025 / exit 1, but the unchanged validator reported all examples valid / exit 0. The C# code path was also checked against upstream 341a526b and is unchanged there.

Changes

  • Launch the Node SDK TypeScript compiler entry point through process.execPath, without a shell, and disable pretty diagnostics.
  • Normalize TypeScript diagnostic paths and retain document locations.
  • Preserve stdout/stderr and reject failed TypeScript or C# builds that produce no failing example result.
  • Quote and escape the Go replacement directory when generating go.mod.
  • Add nine isolated regressions using the real TypeScript compiler, Go module parser, and .NET build tool. Run the suite in the existing Node, Go, and .NET documentation CI steps. No dependency changes.

Validation

  • Windows: Node 24.14.1, Go 1.24.6, .NET SDK 10.0.401; npm --prefix scripts/docs-validation test: 9 passed, 0 skipped.
  • WSL Ubuntu 22.04: Node 22.20.0 and Go 1.24.6, fresh npm ci: the earlier TypeScript/Go suite passed 6 tests, 0 skipped before the C# additions.
  • WSL Ubuntu 22.04: Node 22.20.0, temporary .NET SDK 8.0.425, fresh npm ci; node --test --test-name-pattern=C# validate.test.mjs: 3 passed, 0 skipped after the C# additions. Temporary tooling and fixtures were removed.
  • Before the TypeScript fix, its five tests on Windows gave 1 pass / 4 failures because error scenarios incorrectly exited 0.
  • Before the Go quoting fix, the five TypeScript tests passed and the new Go regression failed when go mod edit -json parsed the actual generated go.mod. Afterward it parses and the replacement path round-trips exactly.
  • Before the C# fix, the expanded suite gave 8 passes / 1 failure: the malformed-project scenario incorrectly exited 0. Afterward all nine pass, including valid C# and CS0029 with its original fixture.md:7 location.
  • Coverage: valid TypeScript in a path containing spaces, TS2322 with document location, global TS2688, missing compiler, errors outside extracted examples, a generated Go replacement path containing spaces (including Windows backslashes), and the three C# scenarios above.
  • git diff --check passed.

Limits

The Go regression checks module generation and parsing, not compilation of the full Go documentation corpus. It skips locally if Go is absent; Go CI supplies Go.

The C# tests build small isolated projects targeting .NET 8 with package sources cleared. They require .NET 8 reference packs to be installed or cached and skip when dotnet is absent. They do not establish that the complete C# documentation corpus compiles.

The full npm run docs:nodejs command still encounters the existing local missing Node type definitions (TS2688). It now exits 1 and prints that diagnostic rather than falsely reporting all examples valid. I am not claiming the full documentation corpus compiles on this machine.

@rinceyuan
rinceyuan requested a review from a team as a code owner October 8, 2026 07:19
Copilot AI balanced review requested due to automatic review settings October 8, 2026 07:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation addresses the false-success paths with focused cross-platform regression coverage.

0 open findings

What changed in this PR

Ensures TypeScript documentation validation reliably fails for compiler startup, global, and external-file errors across platforms.

Changes:

  • Invokes TypeScript through Node and normalizes diagnostic paths.
  • Propagates compilation failures not tied to extracted examples.
  • Adds regression tests and runs them in documentation CI.
File Description
scripts/​docs-validation/​validate.ts Improves compiler execution and failure handling.
scripts/​docs-validation/​validate.test.mjs Adds five real-compiler regression tests.
scripts/​docs-validation/​package.json Adds the test command.
.github/​workflows/​sdk-nodejs.yml Runs validator tests in Linux CI.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@rinceyuan rinceyuan changed the title fix: fail TypeScript docs validation on compiler errors fix: harden TypeScript and Go documentation validation Oct 8, 2026
@rinceyuan rinceyuan changed the title fix: harden TypeScript and Go documentation validation fix: harden TypeScript, Go, and C# documentation validation Oct 9, 2026

This branch has not been deployed

No deployments
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.

2 participants