-
-
Notifications
You must be signed in to change notification settings - Fork 31
feat(DEP0066): OutgoingMessage headers
#281
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
feat(DEP0066): OutgoingMessage headers
#281
Conversation
|
No need to ping me, I receive notification for all thing on the repo |
AugustinMauroy
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.
Not too bad there are some pont that you can improve 👍
| const matches = rootNode.findAll({ | ||
| rule: { pattern: "$OBJ._headers[$KEY]" }, | ||
| }); | ||
| for (const m of matches) { |
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.
| for (const m of matches) { | |
| for (const m of matches) { |
|
|
||
| { | ||
| const matches = rootNode.findAll({ | ||
| rule: { pattern: "$OBJ._headers[$KEY]" }, |
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.
main part here...
Here if the user code have ah propriety expression _headers it's will be catch. We need a more complex logic to catch http usage and -> catch response objects -> catch _headers
I have to admit that the orignal issue examples wasn't 100% complete
|
bump @elvessilvavieira 👋 |
259f795 to
06a18ea
Compare
|
Hi, Augustin I renamed the folder to http-outgoing message-headers then created a function looksLikeOutgoingMessage. pls tell me if it's weak in preventing false positives, or if there are any utilities that could help me. I even considered using resolveBindingPath for when they explicitly use http.createServer, but it has some drawbacks |
Yeah to have better handling of the detection/node catch workflow we can use https://docs.codemod.com/jssg/semantic-analysis which is a brand new feature of codemod BTW codemod have a mcp that can help you it's aware of this new feature |
OutgoingMessage headers
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
JakobJingleheimer
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.
W00t! 🚀
| schema_version: "1.0" | ||
| name: "@nodejs/http-outgoingmessage-headers" | ||
| version: 1.0.0 | ||
| description: Handle DEP0066: migrate deprecated use of `OutgoingMessage.prototype` `_headers` & `_headerNames` to public HTTP header APIs |
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.
Oh, whoops. My bad:
Error: 4:28 syntax error: mapping values are not allowed here (syntax)
I think it doesn't like the :
| description: Handle DEP0066: migrate deprecated use of `OutgoingMessage.prototype` `_headers` & `_headerNames` to public HTTP header APIs | |
| description: Handle DEP0066 - migrate deprecated use of `OutgoingMessage.prototype` `_headers` & `_headerNames` to public HTTP header APIs |
|
The lint is failing on test files. I think we should perhaps ignore test files for linting since we may need to explicitly handle syntax the linter disallows; if so, let's do that in a separate PR and merge it (via https://github.com/nodejs/userland-migrations/actions/runs/21335657538/job/61408018422?pr=281 |
Done here #355 |
JakobJingleheimer
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.
🙌
Hi, @AugustinMauroy
Here's a draft. I'd like to know if I'm on the right track. There are some things missing, like the README, but I'll add them. Thx
Resolves #128