Skip to content

feat(DEP0153): add dns lookup options codemod#473

Open
Herrtian wants to merge 5 commits into
nodejs:mainfrom
Herrtian:dns-lookup-options-coercion
Open

feat(DEP0153): add dns lookup options codemod#473
Herrtian wants to merge 5 commits into
nodejs:mainfrom
Herrtian:dns-lookup-options-coercion

Conversation

@Herrtian
Copy link
Copy Markdown

Refs #409.

Adds a DEP0153 recipe for literal dns.lookup() and dnsPromises.lookup() option values. Dynamic option values are left unchanged for manual review.

Checked with:

  • npm run test --workspace @nodejs/dns-lookup-options-coercion
  • npm run type-check
  • npm run lint
  • git diff --check

Copy link
Copy Markdown
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work ! But cold you add more jsDocs around sub function to know why/when/how is it useful

test suite seem good bases on issue

in readme you said

This recipe only changes literal option values. Dynamic values such as { family: familyOption } need manual review.

what if we use semantic analitic you can found where the option is defined and maybe update it https://docs.codemod.com/jssg/semantic-analysis#api-reference

Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts
@Herrtian
Copy link
Copy Markdown
Author

Herrtian commented May 24, 2026

Added JSDoc around the option rewrite path and noted why identifier propagation is left for manual review.

Checked with:

  • npm run test --workspace @nodejs/dns-lookup-options-coercion
  • npm run type-check
  • npm run lint\n- git diff --check

Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
Copy link
Copy Markdown
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still comment from my previous review that need be addressed

@AugustinMauroy AugustinMauroy added ⚠️ fully-AI-generated awaiting author Reviewer has requested something from the author labels May 27, 2026
@Herrtian
Copy link
Copy Markdown
Author

Updated the lookup call query to use call_expression/function matching, removed the spread path, applied the shorter guards, and switched the value handling to cached kind/switch checks.

Checked with:

  • npm run test --workspace @nodejs/dns-lookup-options-coercion
  • npm run type-check
  • npm run lint
  • git diff --check

Copy link
Copy Markdown
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT!

Comment thread recipes/dns-lookup-options-coercion/src/workflow.ts Outdated
@AugustinMauroy AugustinMauroy added awaiting reviewer Author has responded and needs action from the reviewer and removed awaiting author Reviewer has requested something from the author labels May 28, 2026
@AugustinMauroy AugustinMauroy requested a review from a team May 28, 2026 08:38
Signed-off-by: Herrtian <70463940+Herrtian@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviewer Author has responded and needs action from the reviewer ⚠️ fully-AI-generated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants