Skip to content

[Typescript Fetch] Handle date parameters in the URL path #19313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
macjohnny merged 8 commits into from
Jun 19, 2025
Merged

[Typescript Fetch] Handle date parameters in the URL path #19313

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7747522
Handle date parameters in the URL path
scottaj Aug 6, 2024
2c3501c
Reformat template and add cast for date fields
scottaj Aug 7, 2024
ffa3354
Change code structure by moving path tranformations to assignment
scottaj Aug 20, 2024
78b74c2
Merge remote-tracking branch 'upstream/master' into bugfix/typescript…
scottaj May 2, 2025
244190e
Regenerate samples after merge
scottaj May 2, 2025
373a34c
Updated param checking
scottaj May 9, 2025
9dbfa08
Merge remote-tracking branch 'upstream/master' into bugfix/typescript…
scottaj May 14, 2025
b24c0be
Try a new commit after manually re-logging into CircleCI
scottaj Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/typescript-fetch/apis.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export class {{classname}} extends runtime.BaseAPI {
{{/formParams}}
{{/hasFormParams}}
const response = await this.request({
path: `{{{path}}}`{{#pathParams}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(String(requestParameters['{{paramName}}']))){{/pathParams}},
path: `{{{path}}}`{{#pathParams}}{{#isDateTimeType}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(requestParameters['{{paramName}}'].toISOString())){{/isDateTimeType}}{{^isDateTimeType}}{{#isDateType}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(requestParameters['{{paramName}}'].toISOString().substring(0,10))){{/isDateType}}{{^isDateType}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(String(requestParameters['{{paramName}}']))){{/isDateType}}{{/isDateTimeType}}{{/pathParams}},
Copy link
Copy Markdown
Member

@macjohnny macjohnny Aug 7, 2024

Choose a reason for hiding this comment

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

how about

Suggested change
path: `{{{path}}}`{{#pathParams}}{{#isDateTimeType}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(requestParameters['{{paramName}}'].toISOString())){{/isDateTimeType}}{{^isDateTimeType}}{{#isDateType}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(requestParameters['{{paramName}}'].toISOString().substring(0,10))){{/isDateType}}{{^isDateType}}.replace(`{${"{{baseName}}"}}`, encodeURIComponent(String(requestParameters['{{paramName}}']))){{/isDateType}}{{/isDateTimeType}}{{/pathParams}},
path: `{{{path}}}`.replace(`{${"{{baseName}}"}}`,
{{#pathParams}}
{{#isDateTimeType}}
encodeURIComponent(requestParameters['{{paramName}}'].toISOString())
{{/isDateTimeType}}
{{^isDateTimeType}}
{{#isDateType}}
encodeURIComponent(requestParameters['{{paramName}}'].toISOString().substring(0,10))
{{/isDateType}}
{^isDateType}}
encodeURIComponent(String(requestParameters['{{paramName}}']))
{{/isDateType}}
{{/isDateTimeType}}
{{/pathParams}},

to simplify the template?

moreover, encodeURIComponent(requestParameters['{{paramName}}'].toISOString().substring(0,10)) is problematic because devs might be using a local date, and this converts the date to be in UTC. But at least this matches

openapi-generator/modules/openapi-generator/src/main/resources/typescript-fetch/modelGeneric.mustache

Line 114 in f8a5051

'{{baseName}}': {{^required}}value['{{name}}'] == null ? undefined : {{/required}}({{#required}}{{#isNullable}}value['{{name}}'] == null ? null : {{/isNullable}}{{/required}}(value['{{name}}']{{#isNullable}} as any{{/isNullable}}).toISOString().substring(0,10)),

Copy link
Copy Markdown
Contributor Author

@scottaj scottaj Aug 7, 2024

Choose a reason for hiding this comment

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

I can do the expanded style. I wasn't sure what the standards (if any are for the generated code, so I was trying to maintain the style of that. But your version makes the template more readable.

Regarding the iso string handling, I pretty much just copied the code above that is handling query params (line 140 of this file). So while I don't disagree about the UTC thing, I think if that is an issue it might need to be addressed as part of a larger change

I can add the null handling stuff you have, but are path params allowed to be nullable? I'll admit I don't know the spec well enough to be sure but that seems really weird. I suppose in that case though that putting in null as the path value is the correct behavior?

Copy link
Copy Markdown
Contributor Author

@scottaj scottaj Aug 7, 2024

Choose a reason for hiding this comment

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

Regarding the nullable bit, my reading of the spec (both version 2 and version 3.1.0) is that a path parameter can't be null because the required property must be set to true: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#fixed-fields-10

Determines whether this parameter is mandatory. If the parameter location is "path", this property is REQUIRED and its value MUST be true. Otherwise, the property MAY be included and its default value is false.

Copy link
Copy Markdown
Member

@macjohnny macjohnny Aug 8, 2024

Choose a reason for hiding this comment

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

We can also just add the null-safe operator „.?“ to be cautious

method: '{{httpMethod}}',
headers: headerParameters,
query: queryParameters,
Expand Down