diff --git a/.dagger/modules/e2e/client.dang b/.dagger/modules/e2e/client.dang index bbe9115..38b5786 100644 --- a/.dagger/modules/e2e/client.dang +++ b/.dagger/modules/e2e/client.dang @@ -87,6 +87,10 @@ type ClientChecks { Regenerating a client whose package.json already points @dagger.io/dagger at a local bundle must preserve that dependency (not overwrite it with the engine version pin), and the emitted package.json must be pretty-printed. + + Regeneration owns the `*.gen.ts` bindings and nothing else: hand-written files + in the client package survive, while bindings for a module that has left the + closure are pruned. """ generateClientRespectsExistingCheck(ws: Workspace!): Void @check { let changes = typescriptSdk.generateClient(ws, module: fixtures.clientModule, path: fixtures.clientExisting) @@ -101,6 +105,54 @@ type ClientChecks { ) Asserts.stringContains(pkg, "\"name\": \"my-existing-client\"", "existing package name must be preserved") + # Report what was actually removed: this check has failed on a difference + # between a local-directory workspace and a git-loaded one, and the paths + # are the only way to tell which side of the split behaved differently. + let removed = "removed [" + changes.removedPaths.join(", ") + "]" + Asserts.assert( + Asserts.contains(changes.removedPaths, fixtures.clientExisting + "/main.ts") == false, + "regeneration removed a hand-written file from the client package: " + removed, + ) + Asserts.stringContains( + changes.after.file(fixtures.clientExisting + "/main.ts").contents, + "hello from the user's own file", + "hand-written client code must survive regeneration", + ) + Asserts.assert( + Asserts.contains(changes.removedPaths, fixtures.clientExisting + "/stale-dep.gen.ts"), + "regeneration should prune bindings for a module that left the closure: " + removed, + ) + + null + } + + """ + The ownership split must hold whatever the workspace is made of. + + `Workspace.withNewDirectory` replaces the directory it writes when the + workspace is a local directory and merges into it when the workspace is + synthetic — the shape a git-loaded workspace has, which is how the Cloud + checks runner loads this repo. Pruning that leans on replace therefore stops + pruning in CI while still passing locally, so assert the resulting tree here + rather than the changeset's removals. + """ + generateClientOnSyntheticWorkspaceCheck(ws: Workspace!): Void @check { + let synthetic = ws.directory("/").asWorkspace + let changes = typescriptSdk.generateClient( + synthetic, + module: fixtures.clientModule, + path: fixtures.clientExisting, + ) + let entries = changes.after.directory(fixtures.clientExisting).entries + let kept = "kept [" + entries.join(", ") + "]" + + Asserts.assert(Asserts.contains(entries, "main.ts"), "hand-written client code must survive: " + kept) + Asserts.assert( + Asserts.contains(entries, "stale-dep.gen.ts") == false, + "bindings for a module that left the closure must be pruned: " + kept, + ) + Asserts.assert(Asserts.contains(entries, "dagger.gen.ts"), "regeneration must write the core bindings: " + kept) + null } diff --git a/.dagger/modules/e2e/fixtures/client/existing/main.ts b/.dagger/modules/e2e/fixtures/client/existing/main.ts new file mode 100644 index 0000000..dbae443 --- /dev/null +++ b/.dagger/modules/e2e/fixtures/client/existing/main.ts @@ -0,0 +1,3 @@ +// Hand-written code living alongside the generated client. Regeneration must +// leave it in place. +export const greeting = "hello from the user's own file" diff --git a/.dagger/modules/e2e/fixtures/client/existing/stale-dep.gen.ts b/.dagger/modules/e2e/fixtures/client/existing/stale-dep.gen.ts new file mode 100644 index 0000000..03a2ac5 --- /dev/null +++ b/.dagger/modules/e2e/fixtures/client/existing/stale-dep.gen.ts @@ -0,0 +1,3 @@ +// Bindings for a module that has since left the client's closure. The SDK owns +// the *.gen.ts set, so regeneration must prune this. +export {} diff --git a/.dagger/modules/e2e/init.dang b/.dagger/modules/e2e/init.dang index b7cd63a..8bc4b15 100644 --- a/.dagger/modules/e2e/init.dang +++ b/.dagger/modules/e2e/init.dang @@ -200,6 +200,38 @@ type InitChecks { null } + """ + Init into a directory that already holds files should layer the starter onto + it, never replace it: a user running `dagger module init` inside an existing + project keeps their files, and so does the config the engine wrote at the + destination before calling the SDK. + """ + initOverExistingCheck(ws: Workspace!): Void @check { + let changes = typescriptSdk.initModule(ws, name: "init-over-existing", path: fixtures.lookupModule) + + Asserts.assert( + changes.removedPaths.length == 0, + "init removed existing files: " + changes.removedPaths.join(", "), + ) + Asserts.stringContains( + changes.after.file(fixtures.lookupModule + "/index.ts").contents, + "export class LookupApp", + "init must leave the existing module source in place", + ) + Asserts.stringContains( + changes.after.file(fixtures.lookupModule + "/dagger.json").contents, + "\"source\": \"typescript\"", + "init must leave the existing module config in place", + ) + Asserts.stringContains( + changes.after.file(fixtures.lookupModule + "/src/index.ts").contents, + "export class InitOverExisting", + "init did not render the template into the existing directory", + ) + + null + } + """ Init flags should write configuration into the generated config files. By default, no config keys are written; the runtime decides which file the diff --git a/README.md b/README.md index a2a8479..f2e0ef9 100644 --- a/README.md +++ b/README.md @@ -54,9 +54,10 @@ dagger module init typescript my-module --runtime deno - `node` / `bun` → `package.json`, `tsconfig.json` - `deno` → `deno.json` -If `package.json`, `tsconfig.json`, or `deno.json` already exist at the target -path, init merges Dagger-required keys into them rather than overwriting — your -scripts, path aliases, unstable flags, and other custom settings are preserved. +Init never removes what is already at the target path. If `package.json`, +`tsconfig.json`, or `deno.json` are there, it merges Dagger-required keys into +them rather than overwriting — your scripts, path aliases, unstable flags, and +other custom settings are preserved — and any other file is left untouched. The engine owns the module's config; the SDK only contributes the template and config files above. Run `dagger generate` afterwards to produce the generated @@ -120,6 +121,10 @@ it resolves from any plain client session rather than only from a module runtime. If you point `@dagger.io/dagger` at a local bundle (e.g. `"./sdk"`), regeneration preserves that instead of resetting it to the version pin. +Regeneration owns the `*.gen.ts` files and nothing else: bindings for a module +that has left the closure are dropped, and your own files in the client +directory are left alone. + ## Generate SDK files and clients Regenerate every registered module and client in the workspace: diff --git a/typescript-sdk.dang b/typescript-sdk.dang index f5e63e5..37ad4a3 100644 --- a/typescript-sdk.dang +++ b/typescript-sdk.dang @@ -166,9 +166,10 @@ type TypescriptSdk { The engine resolves the destination `path` and owns the module's config; this function only returns the SDK-owned files to layer onto `path` — the rendered - template plus runtime-specific config. Files already at `path` are merged - into, not overwritten, so an existing package.json / tsconfig.json / deno.json - keeps its scripts, path aliases, and unstable flags. + template plus runtime-specific config. Files already at `path` are kept: an + existing package.json / tsconfig.json / deno.json is merged into rather than + overwritten, so it keeps its scripts, path aliases, and unstable flags, and + everything else there is left alone. """ initModule( ws: Workspace!, @@ -229,7 +230,30 @@ type TypescriptSdk { # `modPath` is workspace-root-relative, so anchor it: a relative workspace # path resolves from ws.cwd and would be prefixed again when initModule is # called directly from a subdirectory rather than driven by the engine. - ws.withNewDirectory("/" + modPath, templateSource).changes(ws) + # + # withNewDirectory replaces the directory it writes, so layer the starter + # onto what is already there: init must never remove a user's files, nor + # the config the engine wrote at `modPath` before calling us. + ws.withNewDirectory( + "/" + modPath, + existingDir(ws, modPath).withDirectory(".", templateSource), + ).changes(ws) + } + } + + """ + Existing contents of a workspace directory, empty when it does not exist yet. + """ + let existingDir(ws: Workspace!, path: String!): Directory! { + if (path == ".") { + ws.directory("/") + } else { + let filtered = ws.directory("/", include: [path + "/**"]) + if (filtered.exists(path)) { + filtered.directory(path) + } else { + directory + } } } @@ -318,7 +342,8 @@ type TypescriptSdk { } else if (runtime == Runtime.BUN) { # The Dagger TypeScript runtime picks bun over node by spotting bun.lock, # so we emit an empty one for a fresh init. If the workspace already has - # one we leave it alone (overlay semantics via init's withDirectory). + # one we leave it alone: init layers the template onto the existing + # directory, so writing it here would truncate the user's lockfile. let nodeConfig = builder .withExec(["config-updator", "package-json", existingPrefix + "package.json", "/rendered/package.json"]) .withExec(["config-updator", "tsconfig", existingPrefix + "tsconfig.json", "/rendered/tsconfig.json"]) @@ -524,6 +549,40 @@ type TypescriptSdk { } } + """ + The `*.gen.ts` bindings sitting at `path`, by name. + + Regeneration owns that set and nothing else: a module that has left the + closure must lose its file, while the user's own files at `path` stay. + """ + let clientBindings(ws: Workspace!, path: String!): [String!]! { + existingDir(ws, path).entries.filter { entry => entry.trimSuffix(".gen.ts") != entry } + } + + """ + Baseline the freshly generated client is layered onto: the client directory as + it stands, minus the bindings regeneration is about to replace. + """ + let existingClientBase(ws: Workspace!, path: String!): Directory! { + existingDir(ws, path).withoutFiles(clientBindings(ws, path)) + } + + """ + The workspace with the bindings at `path` removed. + + Pruning has to happen twice because withNewDirectory does not mean the same + thing for both kinds of workspace: it replaces the directory it writes when + the workspace is a local directory, and merges into it when the workspace is + loaded from git (how the Cloud checks runner loads it). Taking the bindings + out of the baseline covers the first, out of the workspace the second. The + baseline still reads from the untouched `ws`: reading it back out of the + pruned workspace comes up empty on a local directory, which would drop the + user's files. + """ + let withoutClientBindings(ws: Workspace!, path: String!): Workspace! { + clientBindings(ws, path).reduce(ws) { pruned, entry => pruned.withoutFile("/" + path + "/" + entry) } + } + """ Generate a typed client for `module` and stage it at workspace-relative `path`. @@ -546,7 +605,7 @@ type TypescriptSdk { path: String!, ): Changeset! { let modSrc = ws.moduleSource(module) - ws.withNewDirectory("/" + path, clientDirectory( + let generated = clientDirectory( modSrc.clientSchemaIntrospectionJSON.contents, modSrc.moduleOriginalName, modSrc.engineVersion, @@ -557,7 +616,10 @@ type TypescriptSdk { modSrc.asString, modSrc.pin, existingClientConfig(ws, path) - )).changes(ws) + ) + withoutClientBindings(ws, path) + .withNewDirectory("/" + path, existingClientBase(ws, path).withDirectory(".", generated)) + .changes(ws) } """ @@ -610,7 +672,7 @@ type TypescriptSdk { .filter { client => inCwdScope(cwd, client.path) } .reduce(ws) { stagedWs, client => let m = client.moduleSource - stagedWs.withNewDirectory("/" + client.path, clientDirectory( + let generated = clientDirectory( m.clientSchemaIntrospectionJSON.contents, m.moduleOriginalName, m.engineVersion, @@ -621,7 +683,11 @@ type TypescriptSdk { m.asString, m.pin, existingClientConfig(ws, client.path) - )) + ) + withoutClientBindings(stagedWs, client.path).withNewDirectory( + "/" + client.path, + existingClientBase(ws, client.path).withDirectory(".", generated), + ) } .changes(ws) }