Skip to content

Add MPFD Support#10664

Open
jorgerangel-msft wants to merge 3 commits into
microsoft:mainfrom
jorgerangel-msft:feat/mpfd
Open

Add MPFD Support#10664
jorgerangel-msft wants to merge 3 commits into
microsoft:mainfrom
jorgerangel-msft:feat/mpfd

Conversation

@jorgerangel-msft
Copy link
Copy Markdown
Contributor

@jorgerangel-msft jorgerangel-msft commented May 13, 2026

This PR adds support for generating convenience client methods for multipart/form-data request.

contributes to : #9548

@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label May 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@jorgerangel-msft jorgerangel-msft force-pushed the feat/mpfd branch 19 times, most recently from 1984387 to f795535 Compare May 20, 2026 22:45
@jorgerangel-msft jorgerangel-msft force-pushed the feat/mpfd branch 9 times, most recently from b4ca108 to edaf87f Compare May 29, 2026 20:49
@jorgerangel-msft jorgerangel-msft force-pushed the feat/mpfd branch 2 times, most recently from 19689f8 to fd47b14 Compare June 2, 2026 16:10
@jorgerangel-msft jorgerangel-msft requested a review from Copilot June 2, 2026 16:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@jorgerangel-msft jorgerangel-msft force-pushed the feat/mpfd branch 4 times, most recently from 493c81f to cbcc75b Compare June 2, 2026 20:08
Comment thread packages/http-client-csharp/generator/Packages.Data.props Outdated
@jorgerangel-msft jorgerangel-msft changed the title [WIP]: Add MPFD Support Add MPFD Support Jun 4, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 4, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10664

commit: b4407ea

@jorgerangel-msft jorgerangel-msft marked this pull request as ready for review June 4, 2026 17:31
Copy link
Copy Markdown
Contributor

@JoshLove-msft JoshLove-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the production source (emitter TS, Input layer, ClientModel/Core generators). Overall the MPFD design is sound and well-tested. Two inline notes below: one medium correctness concern about mutating a shared cached type in the emitter, and one minor dead-branch nit. Everything else (multipart serialization gating, optional/required handling, isMulti per-element emission, Input converters + Json-usage rule, SCME0004 Experimental gating, constructor augmentation, optional-body MediaType forwarding) looks correct.

--generated by Copilot

Comment thread packages/http-client-csharp/emitter/src/lib/type-converter.ts Outdated
Copy link
Copy Markdown
Contributor

@JoshLove-msft JoshLove-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed after the pr feedback commit (0dcdf53). Both of my earlier comments are resolved — thanks:

  • The dead stream.Position > int.MaxValue branch in MultipartFormDataHelperDefinition.cs is gone; the helpers now return BinaryData.FromBytes(stream.GetWrittenMemory(), mediaType) directly. ✅
  • The shared-bytes cache mutation is fixed by cloning, and the new does not mutate cached bytes types when marking a multipart bytes file part test locks in the invariant (asserting no cached bytes carries isFileType). ✅ Nice regression test.

One residual, inline: the same mutation-of-shared-cache pattern still applies to the model and array-of-model branches, which were not cloned.

--generated by Copilot

Comment thread packages/http-client-csharp/emitter/src/lib/type-converter.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants