Refactor isBinaryPayload to operate on SdkType and drop __raw dependency
- Dominant language
- TypeScript
- Stars
- 27
- Forks
- 90
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 156
Description
## Background
PR Azure/autorest.typescript#3958 introduced an `encode` parameter to `isBinaryPayload` so that `encode === "bytes"` is treated as a binary signal. This is a pragmatic workaround for two related problems but does not fully resolve them.
## Problem 1: `__raw` can be `undefined`
`isBinaryPayload` accepts a raw TypeSpec `Type` and calls `isByteOrByteUnion(dpgContext, body)`, which dereferences `body.kind`.
In the Modular pipeline, callers pass `bodyParameter.__raw!` / `response.type!.__raw!`. Per TCGC's `SdkModelPropertyTypeBase`, `__raw` is **optional** (`__raw?: ModelProperty`). It can be `undefined` for synthesized body parameters such as **spread bodies** (e.g. `createWidget(...CreateWidget)` in `widget_dpg`).
Today this does not crash because the original logic short-circuits:
```typescript
if (type === KnownMediaType.Binary && isByteOrByteUnion(dpgContext, body)) { ... }
```
For JSON content types, `type !== KnownMediaType.Binary` short-circuits before `isByteOrByteUnion(ctx, undefined)` is evaluated. Any future change that breaks this short-circuit (e.g. changing `&&` to `||`) will crash with:
```
TypeError: Cannot read properties of undefined (reading 'kind')
at getEffectiveModelFromType (modelUtils.ts:289)
at getSchemaForType (modelUtils.ts:158)
at isByteOrByteUnion (modelUtils.ts:99)
at isBinaryPayload (operationUtil.ts:208)
at buildBodyParameter (operationHelpers.ts:1772)
```
## Problem 2: `isBinaryPayload` couples payload check with encoding
The function name suggests it checks the payload shape, but with Azure/autorest.typescript#3958 it also peeks at the `encode` metadata. This is a slight semantic coupling — preferably the function should be a pure check on the payload type itself.
## Proposed fix
Refactor `isBinaryPayload` (and the helpers it depends on) to operate on TCGC `SdkType` directly, instead of the raw TypeSpec `Type`. Concretely:
1. Add a helper such as `isSdkBytesType(sdkType: SdkType): boolean` that walks `bytes`, `nullable`, and `union` variants. This replaces `isByteOrByteUnion` for the Modular path.
2. Change the Modular call sites to pass `bodyParameter.type` / `response.type` (`SdkType`) instead of `bodyParameter.__raw` / `response.type.__raw`.
3. Drop the `encode` parameter from `isBinaryPayload` once the function operates on `SdkType` — the `SdkType` itself carries the `encode` metadata, so the function can check it internally as part of "is this payload bytes-on-the-wire".
4. Keep the existing raw-`Type` path for the RLC call site in `transform/transformResponses.ts` (which does not have an `SdkType`), or migrate it separately.
## Benefits
- Removes the latent `__raw === undefined` crash class.
- Restores `isBinaryPayload`'s semantics to "is this payload binary?" without leaking encode-detection into the caller.
- Aligns with the rest of the Modular emitter which already uses `SdkType`.
## Affected call sites
Current usages of `isBinaryPayload` to migrate:
- `packages/typespec-ts/src/modular/helpers/operationHelpers.ts` (3 call sites)
- `packages/typespec-ts/src/transform/transformResponses.ts` (1 call site, RLC path — may keep raw-Type form)
## Related
- PR Azure/autorest.typescript#3958 (workaround that introduced the `encode` parameter)
- Issue Azure/autorest.typescript#3913 (original bug fixed by Azure/autorest.typescript#3958)
Contributor guide
Assessment
This issue has not been assessed yet.