diff --git a/NOTES.md b/NOTES.md index ae8b99d..ffb6027 100644 --- a/NOTES.md +++ b/NOTES.md @@ -2,8 +2,12 @@ Where the task prompt / PR description and the actual code diverged, plus what a production version would need. The authoritative wire structs live in -`internal/contentmapper/` (`hostimpl.go` in particular) and `internal/spanmap/spanmap.go` -at commit `d07c1fff6efd364533b7073dd87b39aaf03029c8`. +`internal/contentmapper/` (`hostimpl.go` in particular) and `internal/spanmap/spanmap.go`. +Originally written against pre-merge commit `d07c1fff6efd364533b7073dd87b39aaf03029c8` +of `andrewbranch/typescript-go#content-mappers`; updated for the **merged** version +(microsoft/typescript-go `main`, merge commit `01b9e721f3d7f8037d700daff94f5808c1afb97e`, +verified at `16c25522`). Protocol version is still **1**. Post-merge changes are marked +"changed at merge" below. ## Divergences: prompt/PR text vs. code (the code won) @@ -14,10 +18,13 @@ at commit `d07c1fff6efd364533b7073dd87b39aaf03029c8`. 2. **Manifest key**: not a top-level `tsContentMapper` field but **`"typescript": { "contentMapper": { ... } }`** in the mapper's `package.json` (`internal/tsoptions/contentmappers.go`). `exec` must be a non-empty string array; - `compilerOptions` (names of compiler options forwarded to `transform`) and - `dynamicConfig` are optional. The package.json **must** declare a `name`; `version` is - folded into the mapper's identity (process sharing + cache keys). The mapper process's - working directory is its package directory. + `compilerOptions` (names of compiler options whose values are folded into the + transform-identity cache key — the option *values* now travel in `openProject`, not + `transform`) and `dynamicConfig` are optional. The package.json **must** declare a + `name`; `version` is folded into the mapper's identity (process sharing + cache keys). + The mapper process's working directory is its package directory. A `contentMappers` + entry in tsconfig may also carry a free-form `options` object, passed through to the + mapper in `openProject` and folded into the identity. 3. **TransformResult field names**: the virtual-syntax field is **`extension`** (`".ts"`, `".tsx"`, `".js"`, `".jsx"`, `".mts"`, `".cts"`, `".mjs"`, `".cjs"`, `".json"`), @@ -33,10 +40,24 @@ at commit `d07c1fff6efd364533b7073dd87b39aaf03029c8`. 5. **Protocol details confirmed in code**: JSON-RPC 2.0 with LSP base-protocol framing (`Content-Length: N\r\n\r\n` + UTF-8 body) over stdio; parent-driven only (a request - from the mapper is a protocol violation); methods `initialize`, `transform`, plus - `openProject`/`closeProject` only if `dynamicConfig: true`. The initialize handshake - has a **5-second timeout**. `diagnosticSource` must be non-empty and must not be - `typescript`, `tsc`, or any native extension name (`ts`, `tsx`, `js`, …). + from the mapper is a protocol violation); methods `initialize`, `openProject`, + `closeProject`, `transform`. The initialize handshake has a **5-second timeout**. + `diagnosticSource` must be non-empty and must not be `typescript`, `tsc`, or any + native extension name (`ts`, `tsx`, `js`, …). + + **Changed at merge — every mapper now gets `openProject`.** Pre-merge (`d07c1ff`), + `openProject`/`closeProject` were sent only to `dynamicConfig` mappers. In the merged + version the host opens a project entry before a mapper's first `transform` in that + project, unconditionally: `openProject` carries `configFileName`, an opaque + `projectHandle`, the entry's `options` from tsconfig, and the project's full effective + `compilerOptions`; `transform` params gained a `projectHandle` field referencing it, + and `closeProject` arrives when the project is released. A non-`dynamicConfig` mapper + **must answer `openProject` with no `configIdentity` and no `watchedFiles`** (`{}` is + the correct reply — a non-empty `configIdentity` from a static mapper is a project + error), and answering "method not found" fails the transform. `initialize` params also + gained an optional `locale` (BCP 47) for mapper-authored diagnostics. This PoC's + server stays a pure function of `(fileName, content)` and just acknowledges + `openProject`/`closeProject` without bookkeeping. 6. **Host-side validation is strict** (worth knowing before it bites): - Verbatim segments are checked character-for-character against both texts. @@ -92,22 +113,21 @@ Both variants demonstrably resolve go-to-definition into the correct selector (R - **Supplemental outputs**: unused here; a `composes`-heavy design might emit shared declarations as a supplemental file instead of repeating them. -## Known trap: launcher-shimmed `node` hangs tsgo at exit - -If `node` on `PATH` is a resident launcher shim (Volta being the known case), -`tsgo` completes the compile but hangs forever in mapper teardown and leaks an -orphaned `node dist/server.js` per run. `childProcess.Close` -(`cmd/tsgo/sys.go`) kills only its direct child — the shim — while the real -node, a reparented grandchild, keeps the inherited stderr-pipe write end open; -`cmd.Wait()` then blocks until that pipe EOFs, which is never. The shim doesn't -forward signals (not even SIGTERM), and tsgo never closes the child's stdin, so -the protocol's own exit-on-EOF path never fires either. - -Workaround: prepend a directory with a symlink to the real binary -(`ln -s "$(volta which node)" dir/node; PATH="dir:$PATH" tsgo ...`). Exec-style -version managers (nvm, asdf) are unaffected. Full diagnosis and suggested -upstream fixes: `UPSTREAM-COMMENT.md` (posted to microsoft/typescript-go#4712, -2026-08-16). +## Resolved upstream: launcher-shimmed `node` used to hang tsgo at exit + +Pre-merge, if `node` on `PATH` was a resident launcher shim (Volta being the +known case), `tsgo` completed the compile but hung forever in mapper teardown +and leaked an orphaned `node dist/server.js` per run: `childProcess.Close` +(`cmd/tsgo/sys.go`) killed only its direct child — the shim — while the real +node, a reparented grandchild, kept the inherited stderr-pipe write end open, +and `cmd.Wait()` blocked until that pipe EOFed, which was never. Reported to +the PR on 2026-08-16 (`UPSTREAM-COMMENT.md`); **fixed in the merged version**: +`Close` now closes the child's stdin first (so a well-behaved mapper exits via +the protocol's exit-on-EOF path even when reparented) and sets +`cmd.WaitDelay = time.Second`, which bounds the reap and yields the tolerated +`exec.ErrWaitDelay` if a descendant still holds the pipes. Regression test: +`TestChildProcessCloseDoesNotWaitForLauncherDescendants` (`cmd/tsgo/sys_unix_test.go`). +No PATH workaround is needed anymore. ## Misc observations diff --git a/README.md b/README.md index 7c72591..be54929 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,8 @@ # css-modules-contentmapper-poc Typed CSS Modules for TypeScript 7 (typescript-go) via the experimental **Content Mapper -API** (PR [microsoft/typescript-go#4712](https://github.com/microsoft/typescript-go/pull/4712)). +API** (PR [microsoft/typescript-go#4712](https://github.com/microsoft/typescript-go/pull/4712), +merged into `main` on 2026-08-19 as `01b9e72`). Importing a `*.module.css` file is type-checked: the imported object's keys are exactly the class names in the CSS, unknown names are compile errors, and go-to-definition jumps from `styles.button` into the `.button` selector. @@ -18,11 +19,11 @@ from `styles.button` into the `.button` selector. ## Running it -Requires a `tsgo` built from the `content-mappers` branch of -`andrewbranch/typescript-go` (this PoC used commit `d07c1ff`, see RESULTS.md): +Requires a `tsgo` built from `main` of `microsoft/typescript-go` (the Content Mapper API +merged in commit `01b9e72`; this PoC last verified against `16c2552`, see RESULTS.md): ```bash -git clone -b content-mappers https://github.com/andrewbranch/typescript-go +git clone https://github.com/microsoft/typescript-go cd typescript-go && go build -o built/tsgo ./cmd/tsgo ``` @@ -35,19 +36,3 @@ npm test -w css-modules-mapper # unit tests path/to/tsgo -p demo --runExternalCode # type-check the demo node scripts/lsp-goto-def.mjs path/to/tsgo demo src/app.ts "button;" # go-to-def ``` - -> [!WARNING] -> If your `node` is a **Volta shim** (`which node` → `~/.volta/bin/node`), -> `tsgo` will hang forever after a successful compile and leak an orphaned -> `node dist/server.js` per run: the shim runs the real node as a grandchild, -> so tsgo's shutdown kill misses it and `tsgo` blocks waiting for the mapper's -> stderr pipe to close. Put the real binary ahead of the shim before running: -> -> ```bash -> mkdir -p /tmp/realnode && ln -sf "$(volta which node)" /tmp/realnode/node -> PATH="/tmp/realnode:$PATH" path/to/tsgo -p demo --runExternalCode -> ``` -> -> Same applies to `scripts/lsp-goto-def.mjs`. Any launcher that keeps the real -> interpreter as a separate resident process triggers this; exec-style managers -> (nvm, asdf) are fine. Details and upstream report: `UPSTREAM-COMMENT.md`. diff --git a/RESULTS.md b/RESULTS.md index a543a63..9b3f3ec 100644 --- a/RESULTS.md +++ b/RESULTS.md @@ -1,14 +1,21 @@ # Acceptance results -PoC of typed CSS Modules via the typescript-go Content Mapper API (PR microsoft/typescript-go#4712). +PoC of typed CSS Modules via the typescript-go Content Mapper API (PR microsoft/typescript-go#4712, +**merged** into `main` on 2026-08-19 as `01b9e721f3d7f8037d700daff94f5808c1afb97e`). ## Environment -- **typescript-go**: branch `content-mappers` of `andrewbranch/typescript-go`, commit **`d07c1fff6efd364533b7073dd87b39aaf03029c8`**, reporting `Version 7.1.0-dev` -- Built with `go build -o built/tsgo ./cmd/tsgo` (Go 1.24.7 toolchain, module toolchain go1.26.0), Node.js v22.22.2 +- **typescript-go**: `main` of `microsoft/typescript-go`, commit **`16c25522e1230b69b11210cfad066d779e6319ba`** (post-merge), reporting `Version 7.1.0-dev` +- Built with `go build -o built/tsgo ./cmd/tsgo` (Go 1.24.7 toolchain), Node.js v22.22.2 - Content mapper protocol version: **1** - `$TSGO` below = the `tsgo` binary built from that commit +All results below were re-run on 2026-08-20 against the merged upstream and are +byte-identical to the original run against pre-merge commit `d07c1ff` of +`andrewbranch/typescript-go#content-mappers`, after one mapper-side change: the merged +host sends `openProject`/`closeProject` to every mapper (not just `dynamicConfig` ones), +so `server.ts` now acknowledges both (see NOTES.md §5). + Setup: ```bash diff --git a/UPSTREAM-COMMENT.md b/UPSTREAM-COMMENT.md index e19c1a0..f0bcebb 100644 --- a/UPSTREAM-COMMENT.md +++ b/UPSTREAM-COMMENT.md @@ -5,6 +5,12 @@ Target: PR [microsoft/typescript-go#4712](https://github.com/microsoft/typescrip `d07c1fff6efd364533b7073dd87b39aaf03029c8`). Posted 2026-08-16; kept here as the record of the diagnosis. +> **Resolved.** The PR merged on 2026-08-19 (`01b9e72`) with this fixed: +> `childProcess.Close` now closes the mapper's stdin before the kill (firing the +> protocol's exit-on-EOF path) and sets `cmd.WaitDelay = time.Second` so the reap +> can't block on pipes held by a reparented descendant, plus a regression test +> (`TestChildProcessCloseDoesNotWaitForLauncherDescendants`). See NOTES.md. + --- ## `tsc` hangs forever at exit when the mapper's `node` is a launcher shim (Volta) diff --git a/css-modules-mapper/src/server.ts b/css-modules-mapper/src/server.ts index b63f36d..952ecf3 100644 --- a/css-modules-mapper/src/server.ts +++ b/css-modules-mapper/src/server.ts @@ -2,9 +2,11 @@ * JSON-RPC 2.0 server over stdio for the typescript-go Content Mapper protocol. * * Framing is the LSP base protocol: "Content-Length: \r\n\r\n". - * TypeScript is the only side that sends requests; this server answers exactly - * "initialize" and "transform" and rejects anything else. It is stateless per - * request — one process may serve many projects in any order. + * TypeScript is the only side that sends requests; this server answers + * "initialize", "openProject", "closeProject", and "transform" and rejects + * anything else. It is stateless per request — one process may serve many + * projects in any order, and transforms are a pure function of + * (fileName, content), so project handles need no bookkeeping here. * * stdout carries protocol frames only; all logging goes to stderr. */ @@ -87,6 +89,16 @@ function handleMessage(body: string): void { }, }); break; + case "openProject": + // Sent before the first transform for each project (since the merged + // protocol; formerly only for dynamicConfig mappers). This mapper has no + // dynamic config, so the result must carry no configIdentity and no + // watchedFiles — an empty object. + send({ jsonrpc: "2.0", id: message.id, result: {} }); + break; + case "closeProject": + send({ jsonrpc: "2.0", id: message.id, result: null }); + break; case "transform": { const params = message.params ?? {}; const fileName = typeof params.fileName === "string" ? params.fileName : "";