From a247b5194c065cfcd35a880b33cf68d349f5e577 Mon Sep 17 00:00:00 2001 From: Vasek - Tom C Date: Tue, 11 Aug 2026 14:38:07 +0200 Subject: [PATCH] feat(config-updator): pin typescript in module config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Module codegen needs the same package.json/tsconfig/deno.json writers init already has — they were the module writers all along, just missing the one thing codegen adds: the typescript pin. It is not cosmetic. The runtime mounts its own prebuilt compiler, and skips dependency installation entirely for an otherwise dependency-free module, only when the pin matches the engine's default; without it every `dagger call` pays for a package-manager install. Deno has no node_modules to fall back on, so it gets the same pin through its import map. A module that picked its own version keeps it, and that means looking in both dependency sections: devDependencies is where a compiler normally goes, and writing dependencies.typescript beside one leaves the module declaring two versions of the same package — which npm resolves to the runtime one, quietly overriding the compiler the user chose. Format everything we write while we are here. sjson edits in place, which reads as "preserve the user's formatting" but only holds for the parts it does not touch: keys it adds are appended compactly, and a file created from scratch — every fresh module — comes out entirely inlined. Indenting reformats an existing file's whitespace, which is a real cost, but these are committed config files people read and edit; emitting them minified is worse. The width stays at pretty's own default rather than zero, so a short array like the @dagger.io/dagger path alias keeps to one line, exactly as the engine's writer leaves it. Two init assertions had encoded the old output shape ("type":"module" with no space) and now read the way the file does. Ignore the helper binaries a bare `go build` drops in a helper directory. Every .dang caller builds them inside a Linux container, so a local one is never read — only committed by accident. Signed-off-by: Tom Chauveau --- .dagger/modules/e2e/init.dang | 8 +-- .gitignore | 8 +++ helpers/config-updator/main.go | 60 +++++++++++++++++---- helpers/config-updator/main_test.go | 81 +++++++++++++++++++++++++++-- 4 files changed, 139 insertions(+), 18 deletions(-) diff --git a/.dagger/modules/e2e/init.dang b/.dagger/modules/e2e/init.dang index 8bc4b15..3ccaa80 100644 --- a/.dagger/modules/e2e/init.dang +++ b/.dagger/modules/e2e/init.dang @@ -142,7 +142,7 @@ type InitChecks { Asserts.stringContains(mergedPkg, "\"name\": \"user-pkg\"", "user-authored name key should be preserved") Asserts.stringContains(mergedPkg, "\"version\": \"1.2.3\"", "user-authored version key should be preserved") Asserts.stringContains(mergedPkg, "\"build\": \"tsc\"", "user-authored script should be preserved") - Asserts.stringContains(mergedPkg, "\"type\":\"module\"", "Dagger-required type=module should be added") + Asserts.stringContains(mergedPkg, "\"type\": \"module\"", "Dagger-required type=module should be added") Asserts.stringNotContains( mergedPkg, "\"@dagger.io/dagger\"", @@ -155,7 +155,7 @@ type InitChecks { Asserts.stringContains(mergedTsConfig, "\"@user/lib\"", "user-authored path mapping should be preserved") Asserts.stringContains( mergedTsConfig, - "\"experimentalDecorators\":true", + "\"experimentalDecorators\": true", "Dagger-required experimentalDecorators should be added", ) Asserts.stringContains(mergedTsConfig, "\"@dagger.io/dagger\"", "Dagger SDK path mapping should be added") @@ -177,10 +177,10 @@ type InitChecks { Asserts.stringContains(mergedDeno, "\"@user/lib\"", "user-authored deno import should be preserved") Asserts.stringContains(mergedDeno, "\"kv\"", "user-authored unstable flag should be preserved") Asserts.stringContains(mergedDeno, "\"@dagger.io/dagger\"", "Dagger SDK import should be added") - Asserts.stringContains(mergedDeno, "\"nodeModulesDir\":\"auto\"", "Dagger-required nodeModulesDir should be added") + Asserts.stringContains(mergedDeno, "\"nodeModulesDir\": \"auto\"", "Dagger-required nodeModulesDir should be added") Asserts.stringContains( mergedDeno, - "\"experimentalDecorators\":true", + "\"experimentalDecorators\": true", "Dagger-required experimentalDecorators should be added", ) Asserts.stringContains(mergedDeno, "\"bare-node-builtins\"", "Dagger-required unstable flag should be appended") diff --git a/.gitignore b/.gitignore index 4fd1256..5561dbd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,11 @@ # Generated client output (dagger generate writes here for the e2e client fixture). .dagger/modules/e2e/fixtures/client/out/ + +# Locally built helper binaries. `go build` in a helper directory defaults the +# output name to the directory's, so a bare `go build` drops one here; every +# .dang caller builds them inside a Linux container instead. +/helpers/codegen/codegen +/helpers/config-updator/config-updator +/helpers/module-config/module-config +/helpers/render-template/render-template diff --git a/helpers/config-updator/main.go b/helpers/config-updator/main.go index 03c67b2..ff681a1 100644 --- a/helpers/config-updator/main.go +++ b/helpers/config-updator/main.go @@ -82,13 +82,17 @@ func run(args []string) error { return fmt.Errorf("%s: %w", subcommand, err) } - out := []byte(updated) - // Client config is emitted fresh into a scoped package, so pretty-print it - // (indented, key order preserved) instead of a single minified line. Module - // variants edit user files in place and keep sjson's format-preserving output. - if strings.HasPrefix(subcommand, "client-") { - out = pretty.PrettyOptions(out, &pretty.Options{Indent: " "}) - } + // Indent every config we write, key order preserved. sjson edits in place, + // which reads as "preserve the user's formatting" but only holds for the + // parts it does not touch: keys it adds are appended compactly, and a file + // created from scratch comes out as a single line. Committed config that + // people read and edit is worth a whole-file reformat. + // + // Width is pretty's own default, not zero, which is what keeps a short array + // on one line. Zero explodes `"@dagger.io/dagger": ["./sdk/index.ts"]` across + // three lines, so the tsconfig.json we write stops matching the engine's byte + // for byte — for two generators that are supposed to agree. + out := pretty.PrettyOptions([]byte(updated), &pretty.Options{Indent: " ", Width: 80}) return os.WriteFile(outputPath, out, 0o644) } @@ -114,6 +118,15 @@ func updatePackageJSON(packageJSON string) (string, error) { return "", fmt.Errorf("set type=module: %w", err) } + // Pin typescript unless the module already chose a version. The runtime + // mounts its own prebuilt copy — and skips dependency installation for an + // otherwise dependency-free module — only when the pin matches the engine's + // default, so drifting from it silently turns every call into an install. + packageJSON, err = pinTypeScript(packageJSON) + if err != nil { + return "", err + } + // Remove legacy in-tree @dagger.io/dagger deps so we transition cleanly to // the engine-managed bundle. Matches dagger/dagger UpdatePackageJSONForModule. for _, key := range []string{ @@ -132,6 +145,26 @@ func updatePackageJSON(packageJSON string) (string, error) { // defaultTypeScriptVersion mirrors dagger/dagger tsdistconsts.DefaultTypeScriptVersion. const defaultTypeScriptVersion = "5.9.3" +// pinTypeScript adds the default typescript pin unless the module already +// declares one, in either dependency section. devDependencies is the normal +// place to put a compiler, so writing dependencies.typescript without looking +// there leaves the module declaring two versions of the same package — npm +// resolves that to the runtime one, quietly overriding the compiler the user +// chose. +func pinTypeScript(packageJSON string) (string, error) { + for _, section := range []string{"dependencies", "devDependencies"} { + if gjson.Get(packageJSON, section+".typescript").Exists() { + return packageJSON, nil + } + } + + packageJSON, err := sjson.Set(packageJSON, "dependencies.typescript", defaultTypeScriptVersion) + if err != nil { + return "", fmt.Errorf("set typescript dependency: %w", err) + } + return packageJSON, nil +} + // updateClientPackageJSON turns the client output dir's package.json into a // self-contained scoped package: it pins @dagger.io/dagger to the module's // engine version, pins typescript, and names the package when unnamed. Existing @@ -164,9 +197,9 @@ func updateClientPackageJSON(packageJSON, engineVersion, moduleName string) (str } } - packageJSON, err = setIfNotExists(packageJSON, "dependencies.typescript", defaultTypeScriptVersion) + packageJSON, err = pinTypeScript(packageJSON) if err != nil { - return "", fmt.Errorf("set typescript dependency: %w", err) + return "", err } return packageJSON, nil @@ -307,7 +340,14 @@ func updateTSConfig(tsConfig string) (string, error) { } func updateDenoConfig(denoConfig string) (string, error) { - denoConfig, err := sjson.Set(denoConfig, "nodeModulesDir", "auto") + // Deno resolves dependencies through this map rather than node_modules, so + // the compiler the module's own code needs has to be declared here. + denoConfig, err := setIfNotExists(denoConfig, "imports.typescript", "npm:typescript@"+defaultTypeScriptVersion) + if err != nil { + return "", fmt.Errorf("set typescript import: %w", err) + } + + denoConfig, err = sjson.Set(denoConfig, "nodeModulesDir", "auto") if err != nil { return "", fmt.Errorf("set nodeModulesDir: %w", err) } diff --git a/helpers/config-updator/main_test.go b/helpers/config-updator/main_test.go index 85c9c9e..054ef26 100644 --- a/helpers/config-updator/main_test.go +++ b/helpers/config-updator/main_test.go @@ -18,7 +18,44 @@ func TestUpdatePackageJSON(t *testing.T) { { name: "empty package.json", packageJSON: `{}`, - expected: `{"type": "module"}`, + expected: `{"type": "module", "dependencies": {"typescript": "5.9.3"}}`, + }, + { + // The runtime mounts its prebuilt compiler only when the pin matches + // its default, so a module that chose its own version keeps it and + // accepts the install rather than being silently retargeted. + name: "a user's own typescript pin is preserved", + packageJSON: `{ + "type": "module", + "dependencies": { + "typescript": "5.4.0" + } +}`, + expected: `{ + "type": "module", + "dependencies": { + "typescript": "5.4.0" + } +}`, + }, + { + // devDependencies is where a compiler normally goes. Adding + // dependencies.typescript beside it would leave two declarations of the + // same package, and npm resolves that to the runtime one — overriding + // the version the module chose. + name: "a user's own typescript devDependency is preserved", + packageJSON: `{ + "type": "module", + "devDependencies": { + "typescript": "5.4.0" + } +}`, + expected: `{ + "type": "module", + "devDependencies": { + "typescript": "5.4.0" + } +}`, }, { name: "package.json with local dagger dependency is stripped", @@ -87,13 +124,16 @@ func TestUpdatePackageJSON(t *testing.T) { "scripts": { "build": "tsc" }, - "type": "module" + "type": "module", + "dependencies": { + "typescript": "5.9.3" + } }`, }, { - name: "type=module already set is a no-op", + name: "type=module already set still gains the typescript pin", packageJSON: `{"type": "module"}`, - expected: `{"type": "module"}`, + expected: `{"type": "module", "dependencies": {"typescript": "5.9.3"}}`, }, } { t.Run(tc.name, func(t *testing.T) { @@ -216,6 +256,7 @@ func TestUpdateDenoConfig(t *testing.T) { denoConfig: `{}`, expected: `{ "imports": { + "typescript": "npm:typescript@5.9.3", "@dagger.io/dagger": "./sdk/index.ts", "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" }, @@ -235,6 +276,7 @@ func TestUpdateDenoConfig(t *testing.T) { name: "deno.json with dagger imports already set is idempotent", denoConfig: `{ "imports": { + "typescript": "npm:typescript@5.9.3", "@dagger.io/dagger": "./sdk/index.ts", "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" }, @@ -251,6 +293,7 @@ func TestUpdateDenoConfig(t *testing.T) { }`, expected: `{ "imports": { + "typescript": "npm:typescript@5.9.3", "@dagger.io/dagger": "./sdk/index.ts", "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" }, @@ -273,6 +316,7 @@ func TestUpdateDenoConfig(t *testing.T) { }`, expected: `{ "imports": { + "typescript": "npm:typescript@5.9.3", "@dagger.io/dagger": "./sdk/index.ts", "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" }, @@ -287,6 +331,33 @@ func TestUpdateDenoConfig(t *testing.T) { "node-globals", "byonm" ] +}`, + }, + { + // Deno has no node_modules to fall back on, so the compiler has to be + // declared here — but a user who picked a version keeps it. + name: "a user's own typescript import is preserved", + denoConfig: `{ + "imports": { + "typescript": "npm:typescript@5.4.0" + } +}`, + expected: `{ + "imports": { + "typescript": "npm:typescript@5.4.0", + "@dagger.io/dagger": "./sdk/index.ts", + "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" + }, + "nodeModulesDir": "auto", + "compilerOptions": { + "experimentalDecorators": true + }, + "unstable": [ + "bare-node-builtins", + "sloppy-imports", + "node-globals", + "byonm" + ] }`, }, { @@ -304,6 +375,7 @@ func TestUpdateDenoConfig(t *testing.T) { "dev": "deno run main.ts" }, "imports": { + "typescript": "npm:typescript@5.9.3", "@user/lib": "./src/lib.ts", "@dagger.io/dagger": "./sdk/index.ts", "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" @@ -329,6 +401,7 @@ func TestUpdateDenoConfig(t *testing.T) { expected: `{ "url": "https://foo/bar/baz.html", "imports": { + "typescript": "npm:typescript@5.9.3", "@dagger.io/dagger": "./sdk/index.ts", "@dagger.io/dagger/telemetry": "./sdk/telemetry.ts" },