-
-
Notifications
You must be signed in to change notification settings - Fork 420
Refactor parameter naming and remove TypeScript error suppressions #1483
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
Conversation
|
- Rename routeInfoByMethodsMap to routesByMethod for clarity - Rename originalRouteName to rawRoute for better readability - Add type validation before paramName.includes() to prevent runtime errors - Add type validation before lodash.camelCase(paramName) to prevent runtime errors - Fix getContentTypes() call to pass required 2 arguments instead of 1 Co-authored-by: smorimoto <38746192+smorimoto@users.noreply.github.com>
|
@copilot Run “yarn fix” and then run “yarn test -u” |
Co-authored-by: smorimoto <38746192+smorimoto@users.noreply.github.com>
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 focuses on code cleanup and type safety improvements by removing TypeScript error suppressions and improving parameter naming. The changes address type checking issues without suppressing errors.
- Renamed parameters for better clarity (
routeInfoByMethodsMap→routesByMethod,originalRouteName→rawRoute) - Added proper type guards for string parameters before using string methods
- Fixed a function call with missing required argument
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: | ||
| typeof paramName === "string" | ||
| ? lodash.camelCase(paramName) | ||
| : lodash.camelCase(String(paramName)), |
Copilot
AI
Oct 30, 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 conditional handling of paramName is redundant. Since paramNames is derived from string operations (splitting and replacing), all elements should already be strings. The typeof paramName === 'string' check and the fallback String(paramName) add unnecessary complexity. Consider simplifying to just lodash.camelCase(paramName) or adding an assertion if type safety is the concern.
| name: | |
| typeof paramName === "string" | |
| ? lodash.camelCase(paramName) | |
| : lodash.camelCase(String(paramName)), | |
| name: lodash.camelCase(paramName), |
| for (const paramName of paramNames) { | ||
| // @ts-expect-error TS(2339) FIXME: Property 'includes' does not exist on type 'unknow... Remove this comment to see the full error message | ||
| if (paramName.includes("-")) { | ||
| if (typeof paramName === "string" && paramName.includes("-")) { |
Copilot
AI
Oct 30, 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 type guard typeof paramName === 'string' appears unnecessary here. The paramNames array is populated from string operations (splitting comma-separated values from regex matches), so all elements should already be strings. This check adds defensive programming but may indicate uncertainty about the data type. Consider simplifying or documenting why non-string values are expected.
| (acc, requestInfo, status) => { | ||
| // @ts-expect-error TS(2554) FIXME: Expected 2 arguments, but got 1. | ||
| const contentTypes = this.getContentTypes([requestInfo]); | ||
| const contentTypes = this.getContentTypes([requestInfo], operationId); |
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.
✅ All Changes Applied Successfully
This PR applies code quality improvements to
src/schema-routes/schema-routes.ts:Parameter Naming Improvements:
routeInfoByMethodsMap→routesByMethodfor better clarity (lines 68-72)originalRouteName→rawRoutefor better readability (lines 94-97, 179)TypeScript Error Fixes:
paramName.includes()to prevent runtime errors (line 160)lodash.camelCase(paramName)to prevent runtime errors (line 166)getContentTypes()to pass required 2 arguments instead of 1 (line 417)Formatting:
Verification:
The changes improve code readability, remove TypeScript error suppressions, and add proper type validation to prevent runtime errors.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Note
Renames parameters for clarity, adds string guards for query param handling, and passes the required second argument to getContentTypes in schema route processing.
createRequestsMapargrouteInfoByMethodsMap→routesByMethodand update internal usage.parseRouteNameargoriginalRouteName→rawRoute; setresult.originalRoutetorawRoute.paramName.includes("-")andlodash.camelCase(paramName)when building query params.getContentTypes([requestInfo], operationId)instead of with a single argument ingetRequestInfoTypes.Written by Cursor Bugbot for commit 1406d6d. This will update automatically on new commits. Configure here.