-
Notifications
You must be signed in to change notification settings - Fork 60
fix: update action type definition #3501
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR improves type safety by expressing the dependency between actions[].type and actions[].executableUser through discriminated unions, ensuring executableUser is required only when type="SECONDARY".
Key Changes:
- Converted
ActionForResponseandActionForParameterto discriminated unions - Made
executableUserrequired fortype="SECONDARY"actions - Removed the unused
ActionTypehelper type
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } & ( | ||
| | { | ||
| type?: "PRIMARY"; | ||
| } | ||
| | { | ||
| type: "SECONDARY"; | ||
| executableUser: { | ||
| entities: ExecutableUserEntityForParameter[]; | ||
| }; | ||
| } | ||
| ); |
Copilot
AI
Nov 19, 2025
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.
The discriminated union for ActionForParameter has a logical flaw. When type is optional on line 94 (type?: "PRIMARY"), it creates ambiguity: if type is undefined, TypeScript cannot determine which union branch applies. This breaks the discriminated union pattern and could allow objects without type or executableUser to pass type checking. Consider making type required for both branches (remove the ? on line 94) to maintain proper type discrimination, or restructure to explicitly handle the case where type is omitted (e.g., by making it a three-way union with an explicit branch for { type?: undefined }).
tasshi-me
left a comment
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.
LGTM!
Why
The dependency relationship between
actions[].typeandactions[].executableUserwas not expressed inthe type system.
In kintone's process management API,
actions[].executableUseris required in requests and included inresponses only when
actions[].typeis"SECONDARY". By expressing this dependency relationship astype information, we can reduce runtime errors and improve the developer experience.
What
ActionForResponsetype to a discriminated union, adding a type constraint that makesexecutableUserrequired only whentype="SECONDARY"ActionForParametertype to a discriminated union, adding a type constraint that makesexecutableUserrequired only whentype="SECONDARY"ActionTypetypeHow to test
Checklist
pnpm lintandpnpm teston the root directory.