-
-
Notifications
You must be signed in to change notification settings - Fork 9
fix bug with res.del when the first arguments it's a function #35
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
transforms/__testfixtures__/v4-deprecated-signatures/send-file.input.ts
Dismissed
Show dismissed
Hide dismissed
transforms/__testfixtures__/v4-deprecated-signatures/send-file.input.ts
Dismissed
Show dismissed
Hide dismissed
transforms/__testfixtures__/v4-deprecated-signatures/send-file.output.ts
Dismissed
Show dismissed
Hide dismissed
transforms/__testfixtures__/v4-deprecated-signatures/send-file.output.ts
Dismissed
Show dismissed
Hide dismissed
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 fixes a bug in the transformer for res.del to ensure that when the first argument is a function it is not modified to res.delete, and it expands test coverage for various response methods.
- Correctly update the transformer for res.del so that only calls with a RegExp, string, or array as the first argument are changed.
- Add new test fixtures for send, sendFile, redirect, json, jsonp, and delete endpoints to capture the updated behavior.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| transforms/v4-deprecated-signatures.ts | Adjusts the call expression search and conditional logic to avoid modifying res.del when the first argument is a function. |
| transforms/testfixtures/v4-deprecated-signatures/send*.ts | Updates tests for res.send to account for separation of status and body. |
| transforms/testfixtures/v4-deprecated-signatures/send-file*.ts | Updates tests to verify the renaming of res.sendfile to res.sendFile. |
| transforms/testfixtures/v4-deprecated-signatures/redirect*.ts | Updates tests to ensure proper parameter order in res.redirect calls. |
| transforms/testfixtures/v4-deprecated-signatures/json*.ts | Updates tests to check the new handling of res.json and res.jsonp calls. |
| transforms/testfixtures/v4-deprecated-signatures/delete*.ts | Adds test cases to verify that calls to app.del with a function argument are preserved. |
Comments suppressed due to low confidence (1)
transforms/v4-deprecated-signatures.ts:107
- [nitpick] Even though the current condition correctly skips transformation for function arguments, it might be clearer and more future-proof to explicitly check for function types (e.g. 'FunctionExpression' or 'ArrowFunctionExpression') to document the intent and avoid accidental modifications if new AST node types are introduced.
if (pathArguments[0].type !== 'RegExpLiteral' && pathArguments[0].type !== 'StringLiteral' && pathArguments[0].type !== 'ArrayExpression')
I found a bug in res.del, because if its first argument is a function, we shouldn't modify it to be res.delete.
And I added more tests to ensure that more cases are covered.