-
Notifications
You must be signed in to change notification settings - Fork 0
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
Allow changing options.type
when patching options
#26
Comments
options.type
when patching optionsoptions.type
when patching options
Just as background, this currently works the way it does because we're deliberately mirroring the way this works in TypeScript. The simple version looks like this: type ShapeOptions = SquareShapeOptions | RoundShapeOptions;
interface SquareShapeOptions {
type: "square",
height: number,
width: number,
}
interface RoundShapeOptions {
type: "round",
radius: number,
} A more complicated example where type ShapeOptions = SquareShapeOptions | RoundShapeOptions | EllipticalShapeOptions;
type RoundishShapeOptions = RoundShapeOptions | EllipticalShapeOptions;
interface SquareShapeOptions {
type: "square",
height: number,
width: number,
}
interface RoundShapeOptions {
type: "round",
radius: number,
}
interface EllipticalShapeOptions {
type: "round",
major: number,
minor: number,
} The key insights here are:
|
Proposal (1), above, would change our output from: RoundShapeOptionsMergePatch:
type: object
required:
- type
properties:
radius:
type: number
type:
type: string
const: round
additionalProperties: false RoundShapeOptionsMergePatch:
type: object
required:
- type
properties:
radius:
type: number
type:
type: string
const: round
additionalProperties:
type: "null" This would allow passing arbitrary other keys, as long as they always contained a |
Right now, if we have a type with:
...and
ShapeOptions
is defined using the following (as supported by #18):...then it is impossible to send a Merge Patch changing
.options
from:...to:
This patch necessary for this conversion looks like:
We need to be able to pass other keys as
null
, so we can remove the originalsquare
parameters when PATCHingtype
.There are several strategies we can use here:
"key": null
. We could do this usingadditionalProperties: { schema: { type: "null" } } }
. But we need to check with @blakew about whether this breaks our doc tooling.height: null
andwidth: null
inRoundShapeOptionsMergePatch
. But this would require generating a special version ofRoundShapeOptionsMergePatch
that knows aboutSquareShapeOptionsMergePatch
. Call it "ShapeOptionsRoundShapeOptionsMergePatch
". But sinceoneOf
is a type union, not inheritance, we might need several versions ofRoundShapeOptionsMergePatch
, one for each type union which includes it. So if we havetype RoundedShapeOptions = RoundShapeOptions | EllipticalShapeOptions
, then we would needRoundedShapeOptionsRoundShapeOptionsMergePatch
andRoundedShapeOptionsEllipticalShapeOptionsMergePatch
, and so on. This is one of those classic bad ideas that just keeps getting worse.RoundShapeOptionsMergePatch
andSquareShapeOptionsMergePatch
directly intoShapeOptionsMergePatch
. This would require a lot of code, but we might be able to make it nice?@hanakslr and @blakew, I'd love your thoughts here.
The text was updated successfully, but these errors were encountered: