Repository navigation
fix(http-client-js): honor custom server option names - #12446
Guishi Wang (guishiw) wants to merge 3 commits into
Conversation
microsoft-github-policy-service
Bot
added
the
emitter:client:js
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the change aligning optional server-parameter declarations with their actual names.
No open issues 🎉
What changed in this PR
Fixes #12352 by making the JavaScript client emitter declare optional server parameters using their actual names, keeping client options consistent with endpoint resolution.
Changes:
- Derive optional server options from client metadata, applying naming and type-emission policies.
- Add coverage for
baseUrland update required-parameter scenarios to omit the unusedendpointoption. - Add a package changelog entry.
| File | Description |
|---|---|
packages/http-client-js/test/scenarios/server/parametrized-endpoint.md |
Removes the unused optional endpoint from expected output. |
packages/http-client-js/test/scenarios/server/multiple-parameters.md |
Updates expected options for required server parameters. |
packages/http-client-js/test/scenarios/server/custom-parameter-name.md |
Tests matching baseUrl declarations and endpoint resolution. |
packages/http-client-js/src/components/client-context/client-context-options.tsx |
Emits named, typed options from optional URL-template parameters. |
.chronus/changes/fix-http-client-js-server-option-name.md |
Documents the JavaScript emitter fix. |
Review effort: Balanced
commit: |
|
All changed packages have been documented.
Show changes
|
Jeff Fisher (xirzec)
left a comment
Jeff Fisher (xirzec)
left a comment
There was a problem hiding this comment.
Changes look good, but could you address the below issue that copilot flagged? Thanks!
| const optionalEndpointParameters = $.client | ||
| .getUrlTemplate(props.client) | ||
| .parameters.filter((parameter) => parameter.optional); |
There was a problem hiding this comment.
Copilot: @server parameters declared with a default value but without ? (e.g., baseUrl: url = "http://localhost:3000") have parameter.optional === false and parameter.defaultValue !== undefined, which buildClientParameters (packages/http-client-js/src/utils/parameters.tsx:19-28 via buildParameterDescriptor) treats as optional and omits from positional constructor arguments. Consider filtering with parameter.optional || parameter.defaultValue !== undefined here (and in ParametrizedEndpoint at packages/http-client-js/src/components/client-context/parametrized-endpoint.tsx:22-24) so default-valued server parameters without ? are included in *ClientOptions and resolved from options.
| const clientOptions: Map<string, Children> = new Map(); | ||
| const optionalEndpointParameters = $.client | ||
| .getUrlTemplate(props.client) | ||
| .parameters.filter((parameter) => parameter.optional || parameter.defaultValue !== undefined); |
Fix: #12352
Root cause
The JavaScript client emitter hardcoded an optional
endpointproperty in the generated client options instead of deriving option properties from the TypeSpec server URL template. As a result, custom optional server parameter names were not represented correctly.Fix strategy
Generate client option properties from the optional URL-template parameters exposed by the client metadata, applying the emitter naming policy and TypeScript type emission. Update the server scenarios and add coverage for a custom optional parameter name while preserving the expected behavior for required and multiple parameters.
The relevant test suite passes with 247 tests passed and 4 skipped. The package build, lint, Prettier, and diff checks also pass.