From df447bbc6cb83b844f871393c284960056fdbeae Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 18:59:57 -0400 Subject: [PATCH 1/2] fix(generate): namespace TypeScript client barrel exports Preserve every binding and facade behind an injective namespace derived from its full relative path, avoiding TS2308 across protobuf packages. Co-Authored-By: Claude Opus 5.5 --- docs/commands.md | 9 ++++ pkg/generators/client_library.go | 24 +++++++++- pkg/generators/client_library_test.go | 65 ++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index e6188cae..245e5f27 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -3434,6 +3434,15 @@ limitation in the facade plugin currently means `--services` filtering does not work over a multi-file descriptor set from these two sources; use the local source, or omit `--services` to generate the full contract's facade. +TypeScript's generated entry point exports each binding and facade as a module +namespace, so equal message names in different proto packages remain distinct. +Names start with `module_` followed by the relative source path without its +extension; non-ASCII-identifier characters are escaped as `$$` +(including `$`). For example, import `{ module_gen$2f$api_pb }` to access +`module_gen$2f$api_pb.Settings`, or `{ module_accounts_facade }` for +`module_accounts_facade.accounts`. Regenerate consumers together with this +entry-point change; direct imports from generated files retain their shape. + ##### Library layout ``` diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index e04d9db7..ba43dd80 100644 --- a/pkg/generators/client_library.go +++ b/pkg/generators/client_library.go @@ -132,7 +132,9 @@ func finalizeTypeScriptLibrary(langDir, npmPackage string) (generatedLanguageExp // src/ is wholly owned by this command (wipeTypeScriptLibrary removes it) and // which files exist depends on the contracts generated — so it is rewritten on // every run instead of written once. Without it the library declares an entry -// point nothing produced and cannot be resolved at all. +// point nothing produced and cannot be resolved at all. Each module gets its +// own namespace: protobuf names are unique within a package, not across the +// dependency graph (two packages may both declare Settings, for example). func writeTypeScriptIndex(langDir string) error { srcDir := filepath.Join(langDir, "src") if err := os.MkdirAll(srcDir, 0o755); err != nil { @@ -162,11 +164,29 @@ func writeTypeScriptIndex(langDir string) error { var content strings.Builder content.WriteString("// generated by codefly generate client; do not edit\n") for _, module := range modules { - fmt.Fprintf(&content, "export * from %q;\n", module) + fmt.Fprintf(&content, "export * as %s from %q;\n", typeScriptModuleNamespace(module), module) } return os.WriteFile(index, []byte(content.String()), 0o644) //nolint:gosec } +// Encode the complete relative path, not just the basename. Escaping every +// non-identifier character (including $ itself) makes this injective, unlike +// replacing punctuation with underscores. The prefix also admits digit-leading +// paths and reserved words without producing an invalid TypeScript identifier. +func typeScriptModuleNamespace(module string) string { + path := strings.TrimSuffix(strings.TrimPrefix(module, "./"), ".js") + var name strings.Builder + name.WriteString("module_") + for _, char := range path { + if char >= 'a' && char <= 'z' || char >= 'A' && char <= 'Z' || char >= '0' && char <= '9' || char == '_' { + name.WriteRune(char) + } else { + fmt.Fprintf(&name, "$%x$", char) + } + } + return name.String() +} + // wipePythonLibrary clears pkgDir's generated content once, before any entry // is merged in: _gen/, which is entirely its own, and each facade at the // package root — a file whose generated-code notice names a protoc plugin, diff --git a/pkg/generators/client_library_test.go b/pkg/generators/client_library_test.go index c3d9fb85..ca12bd6d 100644 --- a/pkg/generators/client_library_test.go +++ b/pkg/generators/client_library_test.go @@ -278,7 +278,7 @@ func TestTypeScriptLibraryDeclaresAnEntryPointItProduces(t *testing.T) { if err != nil { t.Fatalf("src/index.ts, the file tsconfig compiles into the declared entry point, was not written: %v", err) } - for _, want := range []string{`export * from "./gen/api_pb.js";`, `export * from "./pdf_facade.js";`} { + for _, want := range []string{`export * as module_gen$2f$api_pb from "./gen/api_pb.js";`, `export * as module_pdf_facade from "./pdf_facade.js";`} { if !strings.Contains(string(index), want) { t.Errorf("src/index.ts does not re-export %s\ngot:\n%s", want, index) } @@ -302,10 +302,71 @@ func TestTypeScriptLibraryDeclaresAnEntryPointItProduces(t *testing.T) { if err != nil { t.Fatalf("src/index.ts was not rewritten after a wipe: %v", err) } - if !strings.Contains(string(index), `export * from "./gen/other_pb.js";`) { + if !strings.Contains(string(index), `export * as module_gen$2f$other_pb from "./gen/other_pb.js";`) { t.Errorf("the regenerated barrel missed the new module:\n%s", index) } if strings.Contains(string(index), "api_pb") { t.Errorf("the regenerated barrel still re-exports a module the wipe removed:\n%s", index) } } + +// Equal message/service names in distinct proto packages must remain reachable +// without introducing ambiguous exports into the package entry point. +func TestTypeScriptIndexNamespacesEveryModule(t *testing.T) { + langDir := t.TempDir() + modules := []string{ + "gen/codefly/base/v0/work_context_pb", + "gen/host/authority/v1/work_context_pb", + "gen/host/installation/v1/installation_pb", + "gen/saas/accounts/v1/installations_pb", + "gen/saas/composed/org_settings/v1/settings_pb", + "gen/saas/composed/settings/v1/settings_pb", + "gen/a-b", "gen/a_b", "gen/a/b", "gen/a$2f$b", + "default", "1_facade", "é_facade", + } + for _, module := range modules { + path := filepath.Join(langDir, "src", module+".ts") + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("export type Settings = { value: string };\nexport const SettingsSchema = {};\n"), 0o644); err != nil { + t.Fatal(err) + } + } + if err := writeTypeScriptIndex(langDir); err != nil { + t.Fatal(err) + } + indexPath := filepath.Join(langDir, "src", "index.ts") + first, err := os.ReadFile(indexPath) + if err != nil { + t.Fatal(err) + } + seen := map[string]bool{} + for _, line := range strings.Split(string(first), "\n") { + if !strings.HasPrefix(line, "export ") { + continue + } + fields := strings.Fields(line) + if len(fields) != 6 || fields[2] != "as" || fields[4] != "from" { + t.Fatalf("export leaks unqualified symbols: %s", line) + } + name := fields[3] + if seen[name] { + t.Fatalf("duplicate namespace %q", name) + } + seen[name] = true + } + if len(seen) != len(modules) { + t.Fatalf("got %d namespaces for %d modules", len(seen), len(modules)) + } + if err := writeTypeScriptIndex(langDir); err != nil { + t.Fatal(err) + } + second, err := os.ReadFile(indexPath) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Fatal("regeneration changed the barrel") + } +} From a4930d767fe10917481cce99bdabb5290783b778 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:08:29 -0400 Subject: [PATCH 2/2] fix(generate): one namespace per proto package in the TypeScript entry point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit namespaced every generated module by its file path (module_gen$2f$saas$2f$accounts$2f$v1$2f$installations_pb), which made proto file layout part of the TypeScript API and took every flat name out of the package root (review r48 F1, F2). The entry point now has three parts: - each facade re-exported flat under the name its plugin gives it; - one namespace per proto package, named from the package (saas.accounts.v1 -> saas_accounts_v1_pb; an underscore inside a component doubled, so a_b.v1 and a.b_v1 stay distinct), through a barrel in src/packages/.ts — protobuf keeps names unique within a package, so the barrel is unambiguous and moving a message between files of a package renames nothing; - every name only one package declares re-exported flat. A name two packages declare is left out of the flat set and listed, with the namespaces that reach it, in a comment at the top of index.ts. Two facades or package-less files on one name are refused, naming both. Relative to the released flat barrel this removes nothing a consumer could import: the names it drops are exactly the ones `export *` made ambiguous (TS2308; ES excludes them from the module's exports). So this is not a breaking change for service-pdf, module-directory or module-record-store. docs/commands.md no longer claims direct imports from generated files are a consumer path: the scaffolded package.json exports only "." (r48 F3). TestTypeScriptIndexCompiles runs tsc over a colliding fixture under isolatedModules and verbatimModuleSyntax, and checks a colliding name imported flat fails with TS2305 (r48 F4). Co-Authored-By: Claude Opus 5.5 --- docs/commands.md | 38 ++- pkg/generators/client_library.go | 62 ----- pkg/generators/client_library_test.go | 65 +---- pkg/generators/typescript_index.go | 355 ++++++++++++++++++++++++ pkg/generators/typescript_index_test.go | 268 ++++++++++++++++++ 5 files changed, 649 insertions(+), 139 deletions(-) create mode 100644 pkg/generators/typescript_index.go create mode 100644 pkg/generators/typescript_index_test.go diff --git a/docs/commands.md b/docs/commands.md index 245e5f27..d43f1d0c 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -3434,14 +3434,23 @@ limitation in the facade plugin currently means `--services` filtering does not work over a multi-file descriptor set from these two sources; use the local source, or omit `--services` to generate the full contract's facade. -TypeScript's generated entry point exports each binding and facade as a module -namespace, so equal message names in different proto packages remain distinct. -Names start with `module_` followed by the relative source path without its -extension; non-ASCII-identifier characters are escaped as `$$` -(including `$`). For example, import `{ module_gen$2f$api_pb }` to access -`module_gen$2f$api_pb.Settings`, or `{ module_accounts_facade }` for -`module_accounts_facade.accounts`. Regenerate consumers together with this -entry-point change; direct imports from generated files retain their shape. +TypeScript's generated entry point (`src/index.ts`) has three parts. Each facade +is re-exported flat under the name its plugin gives it (`accounts`, +`AccountsClient`). Each proto package is one namespace named from the package — +its components joined by `_`, an underscore inside a component doubled, and +`_pb` appended — so `saas.accounts.v1` is `saas_accounts_v1_pb` and `a_b.v1` +(`a__b_v1_pb`) stays distinct from `a.b_v1` (`a_b__v1_pb`); moving a message +between files of one package renames nothing. Every name only one package +declares is also re-exported flat, so `import { Installation, AuditService }` +keeps working. A name two packages declare (protobuf keeps names unique within a +package, not across the dependency graph) is not exported flat: a comment at the +top of the entry point lists it with each namespace that reaches it, for example +`codefly_base_v0_pb.WorkContextV1` and `host_authority_v1_pb.WorkContextV1`. +Generation refuses, naming both, when two facades or package-less files would +claim one name, since nothing else would reach either. The package barrels live +in `src/packages/.ts`. The scaffolded `package.json` exports only `.`, +so the entry point is a package consumer's only way in; paths under `src/gen/` +are reachable only from inside the library. ##### Library layout @@ -3452,17 +3461,18 @@ libraries/saas-starter-accounts-client/ catalog.codefly.json # the endpoint's catalog entry (subset if --services) contract.binpb | openapi.json go/ go.mod, gen/…, accounts_facade.pb.go - typescript/ package.json, tsconfig.json, src/index.ts, src/gen/…, src/accounts_facade.ts + typescript/ package.json, tsconfig.json, src/index.ts, src/packages/…, src/gen/…, src/accounts_facade.ts python/ pyproject.toml, /_gen/…, /accounts.py ``` The TypeScript library is an ESM package whose root resolves to `dist/index.js` (types `dist/index.d.ts`). `src/index.ts` is generated output, rewritten on -every run: a barrel re-exporting every facade and every generated binding, so -the package name alone reaches every client and message schema. The scaffold -(`package.json`, `tsconfig.json`) is written once and declares a `typescript` -dev dependency and a `build` script (`tsc -p tsconfig.json`) that produces -`dist/` — the only directory, with the README, that the package ships. +every run: a barrel re-exporting every facade and every generated binding +(shaped as above), so the package name alone reaches every client and message +schema. The scaffold (`package.json`, `tsconfig.json`) is written once and +declares a `typescript` dev dependency and a `build` script (`tsc -p +tsconfig.json`) that produces `dist/` — the only directory, with the README, +that the package ships. The Python package root is not a purely generated directory either. A run replaces `/_gen/` whole and, at the root, only the files a protoc plugin's diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index ba43dd80..8cc4b3be 100644 --- a/pkg/generators/client_library.go +++ b/pkg/generators/client_library.go @@ -6,7 +6,6 @@ import ( "context" "fmt" "io" - "io/fs" "os" "os/exec" "path/filepath" @@ -126,67 +125,6 @@ func finalizeTypeScriptLibrary(langDir, npmPackage string) (generatedLanguageExp return generatedLanguageExport{Name: "typescript", Path: "typescript/", Exports: []string{npmPackage}}, nil } -// writeTypeScriptIndex writes src/index.ts: the barrel re-exporting every -// hoisted facade and every generated binding, and so the one file -// package.json's "exports" names. It is generated output, not a scaffold — -// src/ is wholly owned by this command (wipeTypeScriptLibrary removes it) and -// which files exist depends on the contracts generated — so it is rewritten on -// every run instead of written once. Without it the library declares an entry -// point nothing produced and cannot be resolved at all. Each module gets its -// own namespace: protobuf names are unique within a package, not across the -// dependency graph (two packages may both declare Settings, for example). -func writeTypeScriptIndex(langDir string) error { - srcDir := filepath.Join(langDir, "src") - if err := os.MkdirAll(srcDir, 0o755); err != nil { - return err - } - index := filepath.Join(srcDir, "index.ts") - var modules []string - if err := filepath.WalkDir(srcDir, func(path string, entry fs.DirEntry, err error) error { - if err != nil { - return err - } - if entry.IsDir() || path == index || !strings.HasSuffix(entry.Name(), ".ts") { - return nil - } - relative, relErr := filepath.Rel(srcDir, path) - if relErr != nil { - return relErr - } - // The ".js" specifier the generated bindings and facades already use: - // the sources are ESM and resolve by emitted path, not by source name. - modules = append(modules, "./"+strings.TrimSuffix(filepath.ToSlash(relative), ".ts")+".js") - return nil - }); err != nil { - return err - } - sort.Strings(modules) - var content strings.Builder - content.WriteString("// generated by codefly generate client; do not edit\n") - for _, module := range modules { - fmt.Fprintf(&content, "export * as %s from %q;\n", typeScriptModuleNamespace(module), module) - } - return os.WriteFile(index, []byte(content.String()), 0o644) //nolint:gosec -} - -// Encode the complete relative path, not just the basename. Escaping every -// non-identifier character (including $ itself) makes this injective, unlike -// replacing punctuation with underscores. The prefix also admits digit-leading -// paths and reserved words without producing an invalid TypeScript identifier. -func typeScriptModuleNamespace(module string) string { - path := strings.TrimSuffix(strings.TrimPrefix(module, "./"), ".js") - var name strings.Builder - name.WriteString("module_") - for _, char := range path { - if char >= 'a' && char <= 'z' || char >= 'A' && char <= 'Z' || char >= '0' && char <= '9' || char == '_' { - name.WriteRune(char) - } else { - fmt.Fprintf(&name, "$%x$", char) - } - } - return name.String() -} - // wipePythonLibrary clears pkgDir's generated content once, before any entry // is merged in: _gen/, which is entirely its own, and each facade at the // package root — a file whose generated-code notice names a protoc plugin, diff --git a/pkg/generators/client_library_test.go b/pkg/generators/client_library_test.go index ca12bd6d..c3d9fb85 100644 --- a/pkg/generators/client_library_test.go +++ b/pkg/generators/client_library_test.go @@ -278,7 +278,7 @@ func TestTypeScriptLibraryDeclaresAnEntryPointItProduces(t *testing.T) { if err != nil { t.Fatalf("src/index.ts, the file tsconfig compiles into the declared entry point, was not written: %v", err) } - for _, want := range []string{`export * as module_gen$2f$api_pb from "./gen/api_pb.js";`, `export * as module_pdf_facade from "./pdf_facade.js";`} { + for _, want := range []string{`export * from "./gen/api_pb.js";`, `export * from "./pdf_facade.js";`} { if !strings.Contains(string(index), want) { t.Errorf("src/index.ts does not re-export %s\ngot:\n%s", want, index) } @@ -302,71 +302,10 @@ func TestTypeScriptLibraryDeclaresAnEntryPointItProduces(t *testing.T) { if err != nil { t.Fatalf("src/index.ts was not rewritten after a wipe: %v", err) } - if !strings.Contains(string(index), `export * as module_gen$2f$other_pb from "./gen/other_pb.js";`) { + if !strings.Contains(string(index), `export * from "./gen/other_pb.js";`) { t.Errorf("the regenerated barrel missed the new module:\n%s", index) } if strings.Contains(string(index), "api_pb") { t.Errorf("the regenerated barrel still re-exports a module the wipe removed:\n%s", index) } } - -// Equal message/service names in distinct proto packages must remain reachable -// without introducing ambiguous exports into the package entry point. -func TestTypeScriptIndexNamespacesEveryModule(t *testing.T) { - langDir := t.TempDir() - modules := []string{ - "gen/codefly/base/v0/work_context_pb", - "gen/host/authority/v1/work_context_pb", - "gen/host/installation/v1/installation_pb", - "gen/saas/accounts/v1/installations_pb", - "gen/saas/composed/org_settings/v1/settings_pb", - "gen/saas/composed/settings/v1/settings_pb", - "gen/a-b", "gen/a_b", "gen/a/b", "gen/a$2f$b", - "default", "1_facade", "é_facade", - } - for _, module := range modules { - path := filepath.Join(langDir, "src", module+".ts") - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(path, []byte("export type Settings = { value: string };\nexport const SettingsSchema = {};\n"), 0o644); err != nil { - t.Fatal(err) - } - } - if err := writeTypeScriptIndex(langDir); err != nil { - t.Fatal(err) - } - indexPath := filepath.Join(langDir, "src", "index.ts") - first, err := os.ReadFile(indexPath) - if err != nil { - t.Fatal(err) - } - seen := map[string]bool{} - for _, line := range strings.Split(string(first), "\n") { - if !strings.HasPrefix(line, "export ") { - continue - } - fields := strings.Fields(line) - if len(fields) != 6 || fields[2] != "as" || fields[4] != "from" { - t.Fatalf("export leaks unqualified symbols: %s", line) - } - name := fields[3] - if seen[name] { - t.Fatalf("duplicate namespace %q", name) - } - seen[name] = true - } - if len(seen) != len(modules) { - t.Fatalf("got %d namespaces for %d modules", len(seen), len(modules)) - } - if err := writeTypeScriptIndex(langDir); err != nil { - t.Fatal(err) - } - second, err := os.ReadFile(indexPath) - if err != nil { - t.Fatal(err) - } - if string(first) != string(second) { - t.Fatal("regeneration changed the barrel") - } -} diff --git a/pkg/generators/typescript_index.go b/pkg/generators/typescript_index.go new file mode 100644 index 00000000..4c33bf0f --- /dev/null +++ b/pkg/generators/typescript_index.go @@ -0,0 +1,355 @@ +package generators + +import ( + "bufio" + "bytes" + "fmt" + "io/fs" + "os" + "path/filepath" + "regexp" + "sort" + "strings" +) + +// typeScriptPackagesDir holds one generated barrel per proto package, under +// src/ (which this command owns whole), beside gen/ (which the plugins own). +const typeScriptPackagesDir = "packages" + +var ( + // The header protoc-gen-es writes on every file. A file with no package + // clause (or none of this header) declares into protobuf's root scope. + typeScriptProtoPackageRe = regexp.MustCompile(`^// @generated from file .* \(package ([A-Za-z0-9_.]+),`) + // One top-level declaration export: the only export form the protobuf, + // facade and OpenAPI plugins emit. Anything else is refused rather than + // guessed at, since a misread name is an ambiguous or missing export. + typeScriptExportRe = regexp.MustCompile(`^export\s+(?:declare\s+)?(const\s+enum|enum|const|let|var|function\*?|async\s+function|abstract\s+class|class|type|interface|namespace)\s+([A-Za-z_$][\w$]*)`) +) + +// typeScriptModule is one generated file the entry point reaches. +type typeScriptModule struct { + path string // relative to src/, slash-separated, with .ts + facade bool // outside gen/: a hoisted facade + pkg string // proto package; "" for facades and package-less files + exports map[string]bool // exported name -> type-only +} + +// typeScriptOwner is one claimant of a top-level name in the entry point: +// a package's declaration (pkg set), which can yield the flat name to its +// namespace, or anything else (a facade, a namespace, a package-less file), +// which cannot. +type typeScriptOwner struct { + pkg string + label string +} + +func (m typeScriptModule) specifier(fromDir string) string { + rel := strings.TrimSuffix(m.path, ".ts") + ".js" + if fromDir == "" { + return "./" + rel + } + return "../" + rel +} + +// writeTypeScriptIndex writes src/index.ts: the barrel package.json's +// "exports" names. It is generated output, not a scaffold — src/ is wholly +// owned by this command (wipeTypeScriptLibrary removes it) and which files +// exist depends on the contracts generated — so it is rewritten on every run. +// +// Protobuf keeps names unique within a package, not across the dependency +// graph (codefly.base.v0 and saas.accounts.v1 may both declare Settings), so a +// barrel of plain `export *` lines is ambiguous (TS2308). The shape: +// - every facade is re-exported flat, under the name its plugin gave it; +// - every proto package is one namespace named from the package +// (saas.accounts.v1 -> saas_accounts_v1_pb), through a barrel in +// src/packages/, so moving a message between files of a package renames +// nothing; +// - every binding name nothing else claims is also re-exported flat. A +// package's declaration of a name another package, a facade or a +// namespace also claims is left out of the flat set and listed in a +// comment at the top of the barrel; its package namespace reaches it. +// Two facades or package-less files on one name are refused, since +// nothing else would reach either. +func writeTypeScriptIndex(langDir string) error { + srcDir := filepath.Join(langDir, "src") + if err := os.MkdirAll(srcDir, 0o755); err != nil { + return err + } + modules, err := readTypeScriptModules(srcDir) + if err != nil { + return err + } + index, err := newTypeScriptIndex(modules) + if err != nil { + return err + } + if err := index.writePackageBarrels(srcDir); err != nil { + return err + } + return os.WriteFile(filepath.Join(srcDir, "index.ts"), []byte(index.render()), 0o644) //nolint:gosec +} + +// typeScriptIndex is the entry point's plan: which package each binding +// belongs to, each package's namespace, and who claims every top-level name. +type typeScriptIndex struct { + modules []typeScriptModule + packages map[string][]typeScriptModule // proto package -> its files + pkgNames []string // sorted keys of packages + namespaces map[string]string // proto package -> namespace identifier + owners map[string][]typeScriptOwner // top-level name -> its claimants +} + +func newTypeScriptIndex(modules []typeScriptModule) (*typeScriptIndex, error) { + index := &typeScriptIndex{ + modules: modules, + packages: map[string][]typeScriptModule{}, + namespaces: map[string]string{}, + owners: map[string][]typeScriptOwner{}, + } + for _, module := range modules { + if module.pkg != "" { + index.packages[module.pkg] = append(index.packages[module.pkg], module) + } else { + // A facade or a package-less file: its names have no other way in. + for name := range module.exports { + index.claim(name, typeScriptOwner{label: module.path}) + } + } + } + for pkg := range index.packages { + index.pkgNames = append(index.pkgNames, pkg) + } + sort.Strings(index.pkgNames) + for _, pkg := range index.pkgNames { + namespace := typeScriptPackageNamespace(pkg) + index.namespaces[pkg] = namespace + index.claim(namespace, typeScriptOwner{label: "the " + pkg + " namespace"}) + declaredIn := map[string]string{} + for _, module := range index.packages[pkg] { + for name := range module.exports { + if other, dup := declaredIn[name]; dup { + return nil, fmt.Errorf("package %s exports %s from both %s and %s", pkg, name, other, module.path) + } + declaredIn[name] = module.path + index.claim(name, typeScriptOwner{pkg: pkg, label: pkg}) + } + } + } + + // Only a package's declaration can step out of the flat set, because its + // namespace still reaches it. A facade, a namespace and a package-less + // file have no other way in, so two of them on one name is refused. + var refusals []string + for name, claimants := range index.owners { + var fixed []string + for _, owner := range claimants { + if owner.pkg == "" { + fixed = append(fixed, owner.label) + } + } + if len(fixed) > 1 { + sort.Strings(fixed) + refusals = append(refusals, fmt.Sprintf("%s (%s)", name, strings.Join(fixed, ", "))) + } + } + if len(refusals) > 0 { + sort.Strings(refusals) + return nil, fmt.Errorf("the TypeScript entry point cannot export these names unambiguously: %s", strings.Join(refusals, "; ")) + } + return index, nil +} + +func (x *typeScriptIndex) claim(name string, owner typeScriptOwner) { + x.owners[name] = append(x.owners[name], owner) +} + +func (x *typeScriptIndex) collides(name string) bool { return len(x.owners[name]) > 1 } + +func (x *typeScriptIndex) render() string { + var content strings.Builder + content.WriteString("// generated by codefly generate client; do not edit\n") + content.WriteString(x.collisionComment()) + for _, module := range x.modules { + if module.facade { + fmt.Fprintf(&content, "export * from %q;\n", module.specifier("")) + } + } + for _, pkg := range x.pkgNames { + fmt.Fprintf(&content, "export * as %s from %q;\n", x.namespaces[pkg], "./"+typeScriptPackagesDir+"/"+pkg+".js") + } + for _, module := range x.modules { + if !module.facade { + content.WriteString(x.flatExport(module)) + } + } + return content.String() +} + +// collisionComment names every name more than one owner claims, with each +// package namespace that reaches it. +func (x *typeScriptIndex) collisionComment() string { + var collided []string + for name := range x.owners { + if x.collides(name) { + collided = append(collided, name) + } + } + if len(collided) == 0 { + return "" + } + sort.Strings(collided) + var content strings.Builder + content.WriteString("//\n// Names more than one package (or a facade) declares. No package's declaration\n// is re-exported flat under them; reach each through its package namespace:\n") + for _, name := range collided { + var spaces []string + for _, owner := range x.owners[name] { + if owner.pkg != "" { + spaces = append(spaces, x.namespaces[owner.pkg]+"."+name) + } + } + sort.Strings(spaces) + fmt.Fprintf(&content, "// %s: %s\n", name, strings.Join(spaces, ", ")) + } + return content.String() +} + +// flatExport re-exports a binding's names flat: the whole file when none of +// them collide, otherwise each name but the colliding ones, a type-only name +// marked so isolatedModules emits no value import for it. +func (x *typeScriptIndex) flatExport(module typeScriptModule) string { + var flat []string + whole := true + for name, typeOnly := range module.exports { + switch { + case module.pkg != "" && x.collides(name): + whole = false + case typeOnly: + flat = append(flat, "type "+name) + default: + flat = append(flat, name) + } + } + if whole { + return fmt.Sprintf("export * from %q;\n", module.specifier("")) + } + if len(flat) == 0 { + return "" + } + sort.Slice(flat, func(i, j int) bool { + return strings.TrimPrefix(flat[i], "type ") < strings.TrimPrefix(flat[j], "type ") + }) + return fmt.Sprintf("export {\n %s,\n} from %q;\n", strings.Join(flat, ",\n "), module.specifier("")) +} + +// typeScriptPackageNamespace names a proto package's namespace: its +// components joined by "_" with each "_" inside a component doubled, so +// a_b.v1 (a__b_v1_pb) and a.b_v1 (a_b__v1_pb) stay distinct, and the "_pb" +// suffix of protoc-gen-es's own modules, which keeps it clear of the +// facade a single-component package is usually named after (package api, +// facade api). A residual clash is caught as a collision, never emitted. +func typeScriptPackageNamespace(pkg string) string { + components := strings.Split(pkg, ".") + for i, component := range components { + components[i] = strings.ReplaceAll(component, "_", "__") + } + return strings.Join(components, "_") + "_pb" +} + +// writePackageBarrels writes src/packages/.ts per package: +// every file of the package, re-exported whole, which protobuf's uniqueness +// within a package makes unambiguous. The file is named for the package, not +// the namespace, and never ends in _pb.ts: that suffix marks plugin output, +// which consumers hold to the plugin's own stamp. +func (x *typeScriptIndex) writePackageBarrels(srcDir string) error { + dir := filepath.Join(srcDir, typeScriptPackagesDir) + if err := os.RemoveAll(dir); err != nil { + return err + } + if len(x.pkgNames) == 0 { + return nil + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + for _, pkg := range x.pkgNames { + var content strings.Builder + fmt.Fprintf(&content, "// generated by codefly generate client from proto package %s; do not edit\n", pkg) + for _, module := range x.packages[pkg] { + fmt.Fprintf(&content, "export * from %q;\n", module.specifier(typeScriptPackagesDir)) + } + if err := os.WriteFile(filepath.Join(dir, pkg+".ts"), []byte(content.String()), 0o644); err != nil { //nolint:gosec + return err + } + } + return nil +} + +// readTypeScriptModules reads every generated file under srcDir except the +// entry point and the package barrels this command writes, sorted by path. +func readTypeScriptModules(srcDir string) ([]typeScriptModule, error) { + var modules []typeScriptModule + err := filepath.WalkDir(srcDir, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + relative, relErr := filepath.Rel(srcDir, path) + if relErr != nil { + return relErr + } + relative = filepath.ToSlash(relative) + if entry.IsDir() { + if relative == typeScriptPackagesDir { + return filepath.SkipDir + } + return nil + } + if relative == "index.ts" || !strings.HasSuffix(relative, ".ts") { + return nil + } + module, readErr := readTypeScriptModule(path, relative) + if readErr != nil { + return readErr + } + modules = append(modules, module) + return nil + }) + if err != nil { + return nil, err + } + sort.Slice(modules, func(i, j int) bool { return modules[i].path < modules[j].path }) + return modules, nil +} + +func readTypeScriptModule(path, relative string) (typeScriptModule, error) { + data, err := os.ReadFile(path) //nolint:gosec // a file this command just generated + if err != nil { + return typeScriptModule{}, err + } + module := typeScriptModule{ + path: relative, + facade: !strings.HasPrefix(relative, "gen/"), + exports: map[string]bool{}, + } + scanner := bufio.NewScanner(bytes.NewReader(data)) + scanner.Buffer(make([]byte, 0, 64*1024), len(data)+1) + for scanner.Scan() { + line := scanner.Text() + if match := typeScriptProtoPackageRe.FindStringSubmatch(line); match != nil && !module.facade && module.pkg == "" { + module.pkg = match[1] + } + if !strings.HasPrefix(line, "export") { + continue + } + match := typeScriptExportRe.FindStringSubmatch(line) + if match == nil { + return typeScriptModule{}, fmt.Errorf("cannot read the exports of generated %s: %q is not a declaration export", relative, line) + } + typeOnly := match[1] == "type" || match[1] == "interface" + // A name declared as both (an interface merged with a class) is a value. + if previous, seen := module.exports[match[2]]; seen { + typeOnly = typeOnly && previous + } + module.exports[match[2]] = typeOnly + } + return module, scanner.Err() +} diff --git a/pkg/generators/typescript_index_test.go b/pkg/generators/typescript_index_test.go new file mode 100644 index 00000000..e5c6de07 --- /dev/null +++ b/pkg/generators/typescript_index_test.go @@ -0,0 +1,268 @@ +package generators + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// protoModule is a protoc-gen-es shaped binding: its header names the proto +// package, and it declares a message type, its schema and an enum — the +// export forms the real plugin emits — without importing anything, so the +// fixture compiles with no node_modules. +func protoModule(file, pkg string, messages ...string) string { + var b strings.Builder + fmt.Fprintf(&b, "// @generated by protoc-gen-es v2.11.0 with parameter \"target=ts\"\n") + if pkg == "" { + fmt.Fprintf(&b, "// @generated from file %s (syntax proto3)\n", file) + } else { + fmt.Fprintf(&b, "// @generated from file %s (package %s, syntax proto3)\n", file, pkg) + } + b.WriteString("/* eslint-disable */\n\n") + fileConst := "file_" + strings.NewReplacer("/", "_", ".proto", "", "-", "_").Replace(file) + fmt.Fprintf(&b, "export const %s = { name: %q };\n", fileConst, file) + for _, message := range messages { + if strings.HasSuffix(message, "Service") { + fmt.Fprintf(&b, "export const %s = { typeName: %q };\n", message, pkg+"."+message) + continue + } + if strings.HasPrefix(message, "enum ") { + fmt.Fprintf(&b, "export enum %s {\n UNSPECIFIED = 0,\n}\n", strings.TrimPrefix(message, "enum ")) + continue + } + fmt.Fprintf(&b, "export type %s = {\n $typeName: %q;\n value: string;\n};\n", message, pkg+"."+message) + fmt.Fprintf(&b, "export const %sSchema = { typeName: %q };\n", message, pkg+"."+message) + } + return b.String() +} + +func writeTypeScriptFixture(t *testing.T, langDir string, files map[string]string) { + t.Helper() + for path, content := range files { + full := filepath.Join(langDir, "src", filepath.FromSlash(path)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +// collidingTypeScriptLibrary is what TS2308 broke on: equal names declared +// by different proto packages (Settings, WorkContextV1, an enum State), one +// package split across two files, a single-component package named like +// its facade (package api, facade api), and two packages whose names differ +// only in where the underscore sits (a_b.v1, a.b_v1). +func collidingTypeScriptLibrary() map[string]string { + return map[string]string{ + "gen/codefly/base/v0/work_context_pb.ts": protoModule("codefly/base/v0/work_context.proto", "codefly.base.v0", "WorkContextV1", "Settings", "enum State"), + "gen/host/authority/v1/work_context_pb.ts": protoModule("host/authority/v1/work_context.proto", "host.authority.v1", "WorkContextV1", "enum State", "AuthorityService"), + "gen/saas/accounts/v1/installations_pb.ts": protoModule("saas/accounts/v1/installations.proto", "saas.accounts.v1", "Installation", "Settings"), + "gen/saas/accounts/v1/audit_pb.ts": protoModule("saas/accounts/v1/audit.proto", "saas.accounts.v1", "AuditEvent", "AuditService"), + "gen/api_pb.ts": protoModule("api.proto", "api", "Employee", "DirectoryService"), + "gen/a_b/v1/x_pb.ts": protoModule("a_b/v1/x.proto", "a_b.v1", "Left"), + "gen/a/b_v1/x_pb.ts": protoModule("a/b_v1/x.proto", "a.b_v1", "Right"), + "gen/rootless_pb.ts": protoModule("rootless.proto", "", "Rootless"), + "api_facade.ts": `import { DirectoryService } from "./gen/api_pb.js"; + +// generated by protoc-gen-codefly-facade-ts from api; do not edit + +export const api = { + New() { + return { directory: () => DirectoryService }; + }, +}; + +export type ApiClient = ReturnType; +`, + } +} + +// TestTypeScriptIndexNamespacesPackagesAndKeepsUniqueNamesFlat pins the +// entry point's public shape: one namespace per proto package, named from the +// package (not the file, so moving a message between files of a package +// renames nothing), the facade flat, every name only one package declares +// flat, and the colliding names listed instead of exported ambiguously. +func TestTypeScriptIndexNamespacesPackagesAndKeepsUniqueNamesFlat(t *testing.T) { + langDir := t.TempDir() + writeTypeScriptFixture(t, langDir, collidingTypeScriptLibrary()) + if err := writeTypeScriptIndex(langDir); err != nil { + t.Fatal(err) + } + indexPath := filepath.Join(langDir, "src", "index.ts") + first, err := os.ReadFile(indexPath) + if err != nil { + t.Fatal(err) + } + index := string(first) + for _, want := range []string{ + `export * from "./api_facade.js";`, + `export * as api_pb from "./packages/api.js";`, + `export * as a__b_v1_pb from "./packages/a_b.v1.js";`, + `export * as a_b__v1_pb from "./packages/a.b_v1.js";`, + `export * as saas_accounts_v1_pb from "./packages/saas.accounts.v1.js";`, + `export * from "./gen/api_pb.js";`, + `export * from "./gen/saas/accounts/v1/audit_pb.js";`, + `export * from "./gen/rootless_pb.js";`, + "// Settings: codefly_base_v0_pb.Settings, saas_accounts_v1_pb.Settings\n", + "// State: codefly_base_v0_pb.State, host_authority_v1_pb.State\n", + "// WorkContextV1: codefly_base_v0_pb.WorkContextV1, host_authority_v1_pb.WorkContextV1\n", + // A file with a colliding name keeps the rest flat, each type-only + // name marked so isolatedModules emits no value import for it. + "export {\n AuthorityService,\n file_host_authority_v1_work_context,\n} from \"./gen/host/authority/v1/work_context_pb.js\";\n", + "export {\n type Installation,\n InstallationSchema,\n file_saas_accounts_v1_installations,\n} from \"./gen/saas/accounts/v1/installations_pb.js\";\n", + } { + if !strings.Contains(index, want) { + t.Errorf("src/index.ts lacks %q\ngot:\n%s", want, index) + } + } + barrel, err := os.ReadFile(filepath.Join(langDir, "src", "packages", "saas.accounts.v1.ts")) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{`export * from "../gen/saas/accounts/v1/audit_pb.js";`, `export * from "../gen/saas/accounts/v1/installations_pb.js";`} { + if !strings.Contains(string(barrel), want) { + t.Errorf("the saas.accounts.v1 barrel lacks %s\ngot:\n%s", want, barrel) + } + } + + // Regeneration over its own output (packages/ and index.ts present) is a + // fixed point. + if err := writeTypeScriptIndex(langDir); err != nil { + t.Fatal(err) + } + second, err := os.ReadFile(indexPath) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Fatalf("regeneration changed the barrel:\n%s\n---\n%s", first, second) + } +} + +// TestTypeScriptIndexCompiles is the check the original defect needed: TS2308 +// is a compiler error, so only the compiler proves the barrel is unambiguous. +// A consumer reaching every shape — facade, flat names, each colliding name +// through its namespace — must compile under isolatedModules and +// verbatimModuleSyntax, and importing a colliding name flat must not. +func TestTypeScriptIndexCompiles(t *testing.T) { + tsc := typeScriptCompiler(t) + langDir := t.TempDir() + writeTypeScriptFixture(t, langDir, collidingTypeScriptLibrary()) + if _, err := finalizeTypeScriptLibrary(langDir, "@acme/colliding-client"); err != nil { + t.Fatal(err) + } + + consumer := `import { + api, + type ApiClient, + DirectoryService, + type Employee, + AuditService, + type Installation, + InstallationSchema, + type Rootless, + AuthorityService, + codefly_base_v0_pb, + host_authority_v1_pb, + saas_accounts_v1_pb, + a__b_v1_pb, + a_b__v1_pb, +} from "./src/index.js"; + +export const client: ApiClient = api.New(); +export const values = [DirectoryService, AuditService, InstallationSchema, AuthorityService, a__b_v1_pb.LeftSchema, a_b__v1_pb.RightSchema]; +export const employee: Employee = { $typeName: "api.Employee", value: "" }; +export const installation: Installation = { $typeName: "saas.accounts.v1.Installation", value: "" }; +export const rootless: Rootless = { $typeName: ".Rootless", value: "" }; +export const base: codefly_base_v0_pb.WorkContextV1 = { $typeName: "codefly.base.v0.WorkContextV1", value: "" }; +export const host: host_authority_v1_pb.WorkContextV1 = { $typeName: "host.authority.v1.WorkContextV1", value: "" }; +export const settings: saas_accounts_v1_pb.Settings = { $typeName: "saas.accounts.v1.Settings", value: "" }; +export const states = [codefly_base_v0_pb.State.UNSPECIFIED, host_authority_v1_pb.State.UNSPECIFIED]; +` + if out, err := compileTypeScript(t, tsc, langDir, consumer); err != nil { + t.Fatalf("the generated library does not compile for its consumer: %v\n%s\nindex.ts:\n%s", err, out, readFile(t, filepath.Join(langDir, "src", "index.ts"))) + } + + ambiguous := `import { WorkContextV1 } from "./src/index.js"; +export type Picked = WorkContextV1; +` + out, err := compileTypeScript(t, tsc, langDir, ambiguous) + if err == nil || !strings.Contains(out, "TS2305") { + t.Fatalf("importing a name two packages declare flat should fail with TS2305 (no such export), got err=%v:\n%s", err, out) + } +} + +func compileTypeScript(t *testing.T, tsc []string, langDir, consumer string) (string, error) { + t.Helper() + path := filepath.Join(langDir, "consumer.ts") + if err := os.WriteFile(path, []byte(consumer), 0o644); err != nil { + t.Fatal(err) + } + args := append(tsc[1:], "--noEmit", "--strict", "--skipLibCheck", + "--target", "es2022", "--module", "esnext", "--moduleResolution", "bundler", + "--isolatedModules", "--verbatimModuleSyntax", path) + cmd := exec.Command(tsc[0], args...) //nolint:gosec // the test's own compiler invocation + cmd.Dir = langDir + out, err := cmd.CombinedOutput() + return string(out), err +} + +// typeScriptCompiler finds tsc, or fetches the scaffold's pinned TypeScript +// through npx; without node at all the compile check cannot run. +func typeScriptCompiler(t *testing.T) []string { + t.Helper() + if path, err := exec.LookPath("tsc"); err == nil { + return []string{path} + } + if path, err := exec.LookPath("npx"); err == nil { + return []string{path, "--yes", "--package", "typescript@" + pinnedTypeScriptVersion, "tsc"} + } + t.Skip("neither tsc nor npx is on PATH; the TypeScript compile check needs node") + return nil +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +// TestTypeScriptIndexRefusesWhatOnlyAFlatNameReaches: a package's declaration +// can yield its flat name to its namespace; a facade or a package-less file +// cannot, so two of them on one name is a refusal naming both, never a barrel +// that silently drops one. +func TestTypeScriptIndexRefusesWhatOnlyAFlatNameReaches(t *testing.T) { + for name, files := range map[string]map[string]string{ + "two facades": { + "a_facade.ts": "export const shared = {};\n", + "b_facade.ts": "export const shared = {};\n", + }, + "a facade and a package-less file": { + "a_facade.ts": "export const Rootless = {};\n", + "gen/rootless_pb.ts": protoModule("rootless.proto", "", "Rootless"), + }, + "one package declaring a name twice": { + "gen/p/v1/a_pb.ts": protoModule("p/v1/a.proto", "p.v1", "Twice"), + "gen/p/v1/b_pb.ts": protoModule("p/v1/b.proto", "p.v1", "Twice"), + }, + "an export the reader cannot name": { + "gen/p/v1/a_pb.ts": protoModule("p/v1/a.proto", "p.v1", "Fine") + "export { Fine as Alias };\n", + }, + } { + t.Run(name, func(t *testing.T) { + langDir := t.TempDir() + writeTypeScriptFixture(t, langDir, files) + if err := writeTypeScriptIndex(langDir); err == nil { + t.Fatalf("expected a refusal, got:\n%s", readFile(t, filepath.Join(langDir, "src", "index.ts"))) + } + }) + } +}