-
Notifications
You must be signed in to change notification settings - Fork 75
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.
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
remove default value in Modular #1977
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -395,12 +395,9 @@ function getParameterMap( | |
return getConstantValue(param); | ||
} | ||
|
||
if (isOptionalWithouDefault(param)) { | ||
return getOptionalWithoutDefault(param, importSet); | ||
} | ||
|
||
if (isOptionalWithDefault(param)) { | ||
return getOptionalWithDefault(param, importSet); | ||
// if the parameter or property is optional, we don't need to handle the | ||
if (isOptional(param)) { | ||
return getOptional(param, importSet); | ||
} | ||
|
||
if (isRequired(param)) { | ||
|
@@ -476,16 +473,18 @@ function isConstant(param: Parameter | Property): param is ConstantType { | |
); | ||
} | ||
|
||
type OptionalWithoutDefaultType = (Parameter | Property) & { | ||
type: { optional: true; clientDefaultValue: never }; | ||
type OptionalType = (Parameter | Property) & { | ||
type: { optional: true }; | ||
}; | ||
function isOptionalWithouDefault( | ||
|
||
function isOptional( | ||
param: Parameter | Property | ||
): param is OptionalWithoutDefaultType { | ||
return Boolean(param.optional && !param.clientDefaultValue); | ||
): param is OptionalType { | ||
return Boolean(param.optional); | ||
} | ||
function getOptionalWithoutDefault( | ||
param: OptionalWithoutDefaultType, | ||
|
||
function getOptional( | ||
param: OptionalType, | ||
importSet: Map<string, Set<string>> | ||
) { | ||
if (param.type.type === "model") { | ||
|
@@ -498,39 +497,6 @@ function getOptionalWithoutDefault( | |
return `"${param.restApiName}": options?.${param.clientName}`; | ||
} | ||
|
||
type OptionalWithDefaultType = (Parameter | Property) & { | ||
type: { optional: true; clientDefaultValue: string }; | ||
}; | ||
function isOptionalWithDefault( | ||
param: Parameter | Property | ||
): param is OptionalWithDefaultType { | ||
return Boolean(param.clientDefaultValue); | ||
} | ||
|
||
function getOptionalWithDefault( | ||
param: OptionalWithDefaultType, | ||
importSet: Map<string, Set<string>> | ||
) { | ||
if (param.type.type === "model") { | ||
return `"${param.restApiName}": {${getRequestModelMapping( | ||
param.type, | ||
"options?." + param.clientName, | ||
importSet | ||
).join(", ")}} ?? ${getQuotedValue(param)}`; | ||
} | ||
return `"${param.restApiName}": options.${ | ||
param.clientName | ||
} ?? ${getQuotedValue(param)}`; | ||
Comment on lines
-521
to
-523
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the question mark change is because of we now using getOptional which used to be getOptionalWithoutDefault and for OpenAI's case, they have a default value, they previously go into getOptionalWithDefault function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compared with all the places that is using options to build the request body, this is the only places that doesn't have a question mark. As options by literal means this parameter is optional, we should keep |
||
} | ||
|
||
function getQuotedValue(param: OptionalWithDefaultType) { | ||
if (param.type.type === "string") { | ||
return `"${param.clientDefaultValue}"`; | ||
} else { | ||
return param.clientDefaultValue; | ||
} | ||
} | ||
|
||
/** | ||
* Builds the assignment for when a property or parameter has a default value | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the change about question mark