|
| 1 | +--- |
| 2 | +phase: quick-1 |
| 3 | +plan: 01 |
| 4 | +subsystem: codegen |
| 5 | +tags: [typescript, protoc-plugin, query-params, enum, repeated-fields] |
| 6 | + |
| 7 | +# Dependency graph |
| 8 | +requires: |
| 9 | + - phase: 02-shared-annotations |
| 10 | + provides: shared annotations package for query params |
| 11 | +provides: |
| 12 | + - "Fixed enum query param zero-check in TS client (compare against UNSPECIFIED variant)" |
| 13 | + - "Fixed enum query param default/cast in TS server (use UNSPECIFIED + enum type cast)" |
| 14 | + - "Fixed repeated string query param in TS server (getAll instead of get)" |
| 15 | + - "Fixed repeated string query param in TS client (forEach+append instead of set)" |
| 16 | + - "Fixed duplicate const url in TS server for mixed path+query routes" |
| 17 | + - "Fixed unused req parameter in TS client for empty request messages" |
| 18 | + - "TSEnumUnspecifiedValue helper in tscommon for enum zero-value detection" |
| 19 | +affects: [ts-client, ts-server, tscommon] |
| 20 | + |
| 21 | +# Tech tracking |
| 22 | +tech-stack: |
| 23 | + added: [] |
| 24 | + patterns: |
| 25 | + - "TSEnumUnspecifiedValue for enum zero-check in query params" |
| 26 | + - "IsList() early-return pattern for repeated field handling" |
| 27 | + |
| 28 | +key-files: |
| 29 | + created: [] |
| 30 | + modified: |
| 31 | + - "internal/tscommon/types.go" |
| 32 | + - "internal/tsclientgen/generator.go" |
| 33 | + - "internal/tsservergen/generator.go" |
| 34 | + - "internal/httpgen/testdata/proto/query_params.proto" |
| 35 | + |
| 36 | +key-decisions: |
| 37 | + - "Enum zero-check uses first enum value name (UNSPECIFIED variant) from proto definition" |
| 38 | + - "Repeated fields in client use forEach+append for multi-value URL params" |
| 39 | + - "Repeated fields in server use getAll() for proper array extraction" |
| 40 | + - "Server reuses existing url variable when path params already declared it" |
| 41 | + |
| 42 | +patterns-established: |
| 43 | + - "TSEnumUnspecifiedValue: extract UNSPECIFIED enum value with custom enum_value annotation support" |
| 44 | + - "Field.Desc.IsList() early-return before type switch for repeated field handling" |
| 45 | + |
| 46 | +requirements-completed: [BUG-1, BUG-2, BUG-3, BUG-4, BUG-5, BUG-6] |
| 47 | + |
| 48 | +# Metrics |
| 49 | +duration: 6min |
| 50 | +completed: 2026-02-27 |
| 51 | +--- |
| 52 | + |
| 53 | +# Quick Task 1: Fix 6 TS Generator Bugs Summary |
| 54 | + |
| 55 | +**Fixed enum/repeated query param generation, duplicate URL const, and unused req parameter across protoc-gen-ts-client and protoc-gen-ts-server** |
| 56 | + |
| 57 | +## Performance |
| 58 | + |
| 59 | +- **Duration:** 6 min |
| 60 | +- **Started:** 2026-02-27T14:37:41Z |
| 61 | +- **Completed:** 2026-02-27T14:43:41Z |
| 62 | +- **Tasks:** 2 |
| 63 | +- **Files modified:** 11 |
| 64 | + |
| 65 | +## Accomplishments |
| 66 | +- Fixed all 6 TypeScript generation bugs affecting enum query params, repeated fields, duplicate URL declarations, and unused parameters |
| 67 | +- Added TSEnumUnspecifiedValue helper and updated TSZeroCheckForField for enum/repeated field awareness |
| 68 | +- Added test proto cases (Region enum, SearchAdvancedRequest, EmptyRequest) exercising all bug scenarios |
| 69 | +- Updated golden files across all 5 generators (httpgen, clientgen, tsclientgen, tsservergen, openapiv3) |
| 70 | + |
| 71 | +## Task Commits |
| 72 | + |
| 73 | +Each task was committed atomically: |
| 74 | + |
| 75 | +1. **Task 1: Add enum/repeated query param test cases and helper functions** - `cadce9b` (feat) |
| 76 | +2. **Task 2: Fix all 6 bugs in ts-client and ts-server generators** - `a7627b3` (fix) |
| 77 | + |
| 78 | +**Plan metadata:** (pending final commit) |
| 79 | + |
| 80 | +## Files Created/Modified |
| 81 | +- `internal/tscommon/types.go` - Added TSEnumUnspecifiedValue, updated TSZeroCheckForField for enum+repeated, added "enum" case to TSZeroCheck |
| 82 | +- `internal/tsclientgen/generator.go` - Bug #4 (repeated forEach+append), Bug #6 (unused _req prefix) |
| 83 | +- `internal/tsservergen/generator.go` - Bug #2 (enum cast+default), Bug #3 (repeated getAll), Bug #5 (no duplicate const url) |
| 84 | +- `internal/httpgen/testdata/proto/query_params.proto` - Added Region enum, SearchAdvancedRequest, EmptyRequest, two new RPCs |
| 85 | +- `internal/tsclientgen/testdata/golden/query_params_client.ts` - New methods for SearchAdvanced + GetDefaults |
| 86 | +- `internal/tsservergen/testdata/golden/query_params_server.ts` - New handlers for SearchAdvanced + GetDefaults |
| 87 | +- `internal/tsclientgen/testdata/golden/http_verbs_comprehensive_client.ts` - Existing enum field now uses UNSPECIFIED check |
| 88 | +- `internal/tsservergen/testdata/golden/http_verbs_comprehensive_server.ts` - Existing enum field now uses UNSPECIFIED cast |
| 89 | +- `internal/clientgen/testdata/golden/query_params_client.pb.go` - Updated for new RPCs |
| 90 | +- `internal/httpgen/testdata/golden/query_params_http.pb.go` - Updated for new RPCs |
| 91 | +- `internal/openapiv3/testdata/golden/yaml/QueryParamService.openapi.yaml` - Updated for new RPCs |
| 92 | +- `internal/openapiv3/testdata/golden/json/QueryParamService.openapi.json` - Updated for new RPCs |
| 93 | + |
| 94 | +## Decisions Made |
| 95 | +- Enum zero-check uses the first enum value name (the UNSPECIFIED variant) from the proto definition, with custom enum_value annotation support |
| 96 | +- Repeated fields in client use `forEach` + `params.append` for multi-value URL params (not `params.set` with join) |
| 97 | +- Repeated fields in server use `params.getAll()` which returns `string[]` matching the TS interface type |
| 98 | +- Server reuses existing `url` variable when path params already declared it (checks `len(cfg.pathParams) > 0`) |
| 99 | + |
| 100 | +## Deviations from Plan |
| 101 | + |
| 102 | +### Auto-fixed Issues |
| 103 | + |
| 104 | +**1. [Rule 3 - Blocking] Updated golden files for all 5 generators** |
| 105 | +- **Found during:** Task 2 (golden file update) |
| 106 | +- **Issue:** Adding new RPCs to query_params.proto affected all generators that symlink to it (httpgen, clientgen, openapiv3), not just tsclientgen and tsservergen |
| 107 | +- **Fix:** Ran UPDATE_GOLDEN=1 for all affected test packages |
| 108 | +- **Files modified:** clientgen, httpgen, openapiv3 golden files |
| 109 | +- **Verification:** `go test ./...` all pass |
| 110 | +- **Committed in:** a7627b3 (Task 2 commit) |
| 111 | + |
| 112 | +**2. [Rule 3 - Blocking] Applied lint auto-fix for fmt.Fprintf** |
| 113 | +- **Found during:** Task 2 (lint run) |
| 114 | +- **Issue:** golangci-lint auto-fixed `sb.WriteString(fmt.Sprintf(...))` to `fmt.Fprintf(&sb, ...)` in tscommon/types.go |
| 115 | +- **Fix:** Accepted lint auto-fix (correct and more efficient) |
| 116 | +- **Files modified:** internal/tscommon/types.go |
| 117 | +- **Verification:** Tests pass, lint clean (only pre-existing gosec issues in httpgen remain) |
| 118 | +- **Committed in:** a7627b3 (Task 2 commit) |
| 119 | + |
| 120 | +--- |
| 121 | + |
| 122 | +**Total deviations:** 2 auto-fixed (2 blocking) |
| 123 | +**Impact on plan:** Both auto-fixes necessary for test correctness. No scope creep. |
| 124 | + |
| 125 | +## Issues Encountered |
| 126 | +- Xcode license not agreed to on this machine, preventing `make build` and `make lint-fix` from working. Worked around by calling `go build` and `golangci-lint` directly. |
| 127 | +- Pre-existing gosec warnings in httpgen/generator.go (integer overflow conversion) unrelated to this task. |
| 128 | + |
| 129 | +## User Setup Required |
| 130 | +None - no external service configuration required. |
| 131 | + |
| 132 | +## Next Phase Readiness |
| 133 | +- All TS generators now produce correct TypeScript for enum query params, repeated fields, mixed path+query routes, and empty requests |
| 134 | +- Ready for Phase 8+ language work |
| 135 | + |
| 136 | +--- |
| 137 | +*Quick Task: 1-fix-6-ts-generator-bugs-enum-query-param* |
| 138 | +*Completed: 2026-02-27* |
0 commit comments