action params type fix #23
Conversation
Generate changelog in
|
| @@ -45,21 +45,37 @@ export interface ValidLegacyActionParameterTypes { | |||
| export type ActionArgs< | |||
| O extends OntologyDefinition<any>, | |||
| A extends keyof O["actions"], | |||
| > = | |||
| & { | |||
| > = NonNullableKeys<O["actions"][A]["parameters"]> extends never ? { | |||
There was a problem hiding this comment.
Basically, the problem was that our types as they stand allowed you to pass in arrays to action args (based on our tests atleast). It was because if NonNullableKeys or NullableKeys were never, the type would look something like {nullableKeys} & {}, and for some reason having that & {} just allowed users to put in anything. Attempted to fix that here, but there may be a cleaner solution for this
There was a problem hiding this comment.
Specifically {[P in never]: any} accepts []
| @@ -81,6 +80,10 @@ describe("Actions", () => { | |||
| ] | |||
| >(); | |||
|
|
|||
| expectTypeOf<typeof actions.createTask>() | |||
There was a problem hiding this comment.
Not really sure if this is the best thing to do here. Using toBeCallableWith was the only way I could get the arrays to pass through when we had the incorrect types, unfortunately, you can't use .not to check that we don't regress this behavior, so I put in expect-error, which should flag an error if it actually is callable with the array.
There was a problem hiding this comment.
Yeah I think this as good as we can get for now. Once you add bulk actions support this test is going to just get deleted I guess anyhow.
| type Result, | ||
| ReturnEditsMode, | ||
| import { ActionExecutionMode, ReturnEditsMode } from "../.."; | ||
| import type { |
There was a problem hiding this comment.
This happened automatically on save, not sure if this is what we want
There was a problem hiding this comment.
🤷 unless you want to propose changes to dprint's config its fine by me
| type Result, | ||
| ReturnEditsMode, | ||
| import { ActionExecutionMode, ReturnEditsMode } from "../.."; | ||
| import type { |
There was a problem hiding this comment.
🤷 unless you want to propose changes to dprint's config its fine by me
| @@ -81,6 +80,10 @@ describe("Actions", () => { | |||
| ] | |||
| >(); | |||
|
|
|||
| expectTypeOf<typeof actions.createTask>() | |||
There was a problem hiding this comment.
Yeah I think this as good as we can get for now. Once you add bulk actions support this test is going to just get deleted I guess anyhow.
No description provided.