FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Revert the change of network interfaces family from string to integer by aduh95 · Pull Request #43054 · nodejs/node · GitHub

/ node Public

Revert the change of network interfaces family from string to integer - #43054

Closed
aduh95 wants to merge 5 commits into
nodejs:mainfrom
aduh95:revert-net-family-integer
ConversationCommits (5)ChecksFiles changed
Closed

Revert the change of network interfaces family from string to integer#43054
aduh95 wants to merge 5 commits into
nodejs:mainfrom
aduh95:revert-net-family-integer

Conversation

aduh95 commented May 11, 2022

Copy link
Copy Markdown
Contributor

This reverts commits 68fb0bf, 4ad342a, and 3a26db9.

Fixes: #43014

nodejs-github-bot commented May 11, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 11, 2022
aduh95 added semver-major PRs that contain breaking changes and should be released in the next major version. tsc-agenda Issues and PRs to discuss during the meetings of the TSC. labels May 11, 2022

targos commented May 11, 2022

Copy link
Copy Markdown
Member

IMO the revert doesn't make sense if it is semver-major, because everyone will have to adapt to the change for v18 (which is going to be LTS).

mcollina approved these changes May 11, 2022
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

mcollina approved these changes May 11, 2022
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

mcollina commented May 11, 2022

Copy link
Copy Markdown
Member

+1 to land it on Node v18.

mcollina added semver-major PRs that contain breaking changes and should be released in the next major version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels May 11, 2022

mcollina commented May 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

@nodejs/tsc at Today meeting we did not have quorum to LGTM as a non-semver-major change. Does anybody from the TSC objects to this?

We actually had quorum at the end of the meeting, we approve landing this as semver-minor.

mcollina added semver-minor PRs that contain new features and should be released in the next minor version. dont-land-on-v12.x and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels May 11, 2022
RaisinTen added the notable-change PRs with changes that should be highlighted in changelogs. label May 11, 2022
RaisinTen approved these changes May 11, 2022
View reviewed changes
Comment thread lib/dns.js Outdated

RaisinTen commented May 11, 2022

Copy link
Copy Markdown
Member

Is it okay if we remove this from the tsc-agenda now, since everyone in the meeting was +1 with reverting?

aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed backport-requested-v18.x labels May 11, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 11, 2022

This comment was marked as outdated.

Sign in to view
Trott approved these changes May 11, 2022
View reviewed changes

Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Rubber stamp

nodejs-github-bot commented Jun 6, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44397/

nodejs-github-bot commented Jun 7, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44401/

nodejs-github-bot commented Jun 8, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44409/

github-actions Bot mentioned this pull request Jun 13, 2022
Open
15 tasks
mhdawson mentioned this pull request Jun 15, 2022
Closed

mhdawson commented Jun 15, 2022

Copy link
Copy Markdown
Member

We discussed in the TSC meeting and there were no objections to this landing. Doing that now.

mhdawson pushed a commit that referenced this pull request Jun 15, 2022
dns: accept 'IPv4' and 'IPv6' for family
70b516e 
Refs: #43014

PR-URL: #43054
Fixes: #43014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

mhdawson commented Jun 15, 2022

Copy link
Copy Markdown
Member

Landed as 70b516e

mhdawson closed this Jun 15, 2022
aduh95 mentioned this pull request Jun 15, 2022
Merged
danielleadams pushed a commit that referenced this pull request Jun 16, 2022
dns: accept 'IPv4' and 'IPv6' for family
0bb84b0 
Refs: #43014

PR-URL: #43054
Fixes: #43014
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
danielleadams added a commit that referenced this pull request Jun 16, 2022
2022-06-16, Version 18.4.0 (Current)
e339e9c 
Notable changes:

* crypto:
  * remove Node.js-specific webcrypto extensions (Filip Skokan) #43310
  * add CFRG curves to Web Crypto API (Filip Skokan) #42507
* dns:
  * accept `'IPv4'` and `'IPv6'` for `family` (Antoine du Hamel) #43054
* report:
  * add more heap infos in process report (theanarkh) #43116

PR-URL: #43385
danielleadams added a commit that referenced this pull request Jun 16, 2022
2022-06-16, Version 18.4.0 (Current)
f50a80c 
Notable changes:

* crypto:
  * remove Node.js-specific webcrypto extensions (Filip Skokan) #43310
  * add CFRG curves to Web Crypto API (Filip Skokan) #42507
* dns:
  * accept `'IPv4'` and `'IPv6'` for `family` (Antoine du Hamel) #43054
* report:
  * add more heap infos in process report (theanarkh) #43116

PR-URL: #43385
chitowncrispy mentioned this pull request Jul 23, 2022
Closed
1 task
AdamMajer mentioned this pull request Jul 28, 2022
Closed
aduh95 deleted the revert-net-family-integer branch March 29, 2023 16:00
aduh95 mentioned this pull request Mar 29, 2023
Merged
sapphi-red mentioned this pull request Aug 10, 2023
Merged
9 tasks
aduh95 mentioned this pull request Sep 16, 2024
Closed
nrako added a commit to nrako/deno that referenced this pull request Nov 30, 2025
fix(node/net): return string family in server.address()
ebd08d6 
Aligns Deno's `server.address()` with Node.js v18.4.0+ behavior by
returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due
to ecosystem breakage (nodejs/node#43014).

This fix ensures compatibility with npm packages that rely on the
string format, which has been the stable API since Node.js v18.4.0.

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nrako mentioned this pull request Nov 30, 2025
Merged
nrako added a commit to nrako/deno that referenced this pull request Nov 30, 2025
fix(node/net): return string family in server.address()
618980d 
Aligns Deno's `server.address()` with Node.js v18.4.0+ behavior by
returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due
to ecosystem breakage (nodejs/node#43014).

This fix ensures compatibility with npm packages that rely on the
string format, which has been the stable API since Node.js v18.4.0.

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nrako added a commit to nrako/deno that referenced this pull request Nov 30, 2025
fix(node/net): return string family in server.address()
6ba8a42 
Aligns Deno's `server.address()` with Node.js v18.4.0+ behavior by
returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due
to ecosystem breakage (nodejs/node#43014).

This fix ensures compatibility with npm packages that rely on the
string format, which has been the stable API since Node.js v18.4.0.

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nrako added a commit to nrako/deno that referenced this pull request Nov 30, 2025
fix(node/net): return string family in server.address()
b7c45b3 
Aligns Deno's `server.address()` with Node.js v18.4.0+ behavior by
returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due
to ecosystem breakage (nodejs/node#43014).

This fix ensures compatibility with npm packages that rely on the
string format, which has been the stable API since Node.js v18.4.0.

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nrako added a commit to nrako/deno that referenced this pull request Nov 30, 2025
fix(node/net): return string family in server.address()
fd3260a 
Aligns Deno's `server.address()` with Node.js v18.4.0+ behavior by
returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due
to ecosystem breakage (nodejs/node#43014).

This fix ensures compatibility with npm packages that rely on the
string format, which has been the stable API since Node.js v18.4.0.

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nrako added a commit to nrako/deno that referenced this pull request Nov 30, 2025
fix(node/net): return string family in server.address()
7fe7995 
Aligns Deno's Node.js compatibility layer with Node.js v18.4.0+ behavior
by returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number in `server.address()` and socket address methods.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due to
ecosystem breakage (nodejs/node#43014).

Changes:
- `http.Server.address()`, `https.Server.address()`, `http2.Server.address()`
  now include `family` property as string
- `net.Server.address()` returns string `family` via tcp_wrap.ts
- `socket.remoteFamily` returns string instead of number
- `dns.lookup()` accepts both numeric (0, 4, 6) and string ("IPv4", "IPv6")
  family values, matching Node.js behavior

Family value derivation follows Node.js C++ implementation:
- isIP() === 4 → "IPv4"
- isIP() === 6 → "IPv6"
- isIP() === 0 → undefined (defensive, matches Node.js behavior for
  non-IP addresses, though not practically reachable via TCP server APIs)

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nrako added a commit to nrako/deno that referenced this pull request Dec 4, 2025
fix(node/net): return string family in server.address()
6911007 
Aligns Deno's Node.js compatibility layer with Node.js v18.4.0+ behavior
by returning the `family` property as a string ("IPv4" or "IPv6") rather
than a number in `server.address()` and socket address methods.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431), but reverted in v18.4.0 (nodejs/node#43054) due to
ecosystem breakage (nodejs/node#43014).

Changes:
- `http.Server.address()`, `https.Server.address()`, `http2.Server.address()`
  now include `family` property as string
- `net.Server.address()` returns string `family` via tcp_wrap.ts
- `socket.remoteFamily` returns string instead of number
- `dns.lookup()` accepts both numeric (0, 4, 6) and string ("IPv4", "IPv6")
  family values, matching Node.js behavior

Family value derivation follows Node.js C++ implementation:
- isIP() === 4 → "IPv4"
- isIP() === 6 → "IPv6"
- isIP() === 0 → undefined (defensive, matches Node.js behavior for
  non-IP addresses, though not practically reachable via TCP server APIs)

Refs:
- nodejs/node#43054
- nodejs/node@70b516e

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Tango992 added a commit to denoland/deno that referenced this pull request Dec 6, 2025
fix(node/net): return string family in server.address() (#31465)
bc9d356 
## Summary

Aligns Deno's Node.js compatibility layer (`node:net`, `node:http`,
`node:https`,
`node:http2`, `node:dns`) with Node.js v18.4.0+ behavior by returning
the `family`
property as a string (`"IPv4"` or `"IPv6"`) rather than a number in
`server.address()`
   and socket address methods.

Node.js briefly changed `family` from string to number in v18.0.0
(nodejs/node#41431),
   but reverted in v18.4.0 (nodejs/node#43054) due to ecosystem breakage
   (nodejs/node#43014).

This fix ensures compatibility with npm packages that rely on the string
format, which
   has been the stable API since Node.js v18.4.0.

   ## Changes

- Modified `ext/node/polyfills/http.ts` to add `family` property to
`address()` return
- Modified `ext/node/polyfills/internal_binding/tcp_wrap.ts` to return
string `family`
instead of number in `getsockname()`, `getpeername()`, and `#connect()`
- Modified `ext/node/polyfills/net.ts` to fix `socket.remoteFamily`
getter (no longer
   needs conversion since `family` is now a string)
   - Modified `ext/node/polyfills/dns.ts` and
`ext/node/polyfills/internal/dns/promises.ts` to accept string family
values (`"IPv4"`,
`"IPv6"`) in `lookup()`, matching [Node.js
behavior](https://nodejs.org/api/dns.html#dnslookuphostname-options-callback)
- Added tests in `tests/unit_node/http_test.ts`,
`tests/unit_node/http2_test.ts`,
   `tests/unit_node/https_test.ts`, `tests/unit_node/dns_test.ts`, and
   `tests/unit_node/net_test.ts`

   ## Node.js Compatibility Note

For non-IP addresses (when `isIP()` returns 0), the `family` property is
`undefined`.
This matches Node.js C++ behavior in
[`AddressToJS`](https://github.com/nodejs/node/blob/main/src/tcp_wrap.cc)
where family
is only set for `AF_INET` (`"IPv4"`) and `AF_INET6` (`"IPv6"`), and left
undefined
   otherwise.

   ## Refs

   - nodejs/node#43054
   - nodejs/node@70b516e


<!--
Before submitting a PR, please read
https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't
run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench'
label.
-->

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Daniel Rahmanto <daniel.rahmanto@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
Trott Trott approved these changes
BethGriggs BethGriggs approved these changes
BridgeAR BridgeAR approved these changes
RaisinTen RaisinTen approved these changes
+2 more reviewers
targos targos approved these changes
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version. tsc-agenda Issues and PRs to discuss during the meetings of the TSC.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Revert the change of network interfaces family from String to Integer
Back | FazBrowse Home | New Git URL