Skip to content
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

Expect to expose getDefaultVerbatim method #46919

Closed
btea opened this issue Mar 3, 2023 · 9 comments · Fixed by #46973
Closed

Expect to expose getDefaultVerbatim method #46919

btea opened this issue Mar 3, 2023 · 9 comments · Fixed by #46973
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@btea
Copy link
Contributor

btea commented Mar 3, 2023

What is the problem this feature will solve?

When the dnsOrder value of the dns module changes, I hope to get the modified value or judge that the value has changed. Therefore, I want to expose getDefaultVerbatim method or expose a new method getDefaultResultOrder.

function getDefaultVerbatim() {
return dnsOrder !== 'ipv4first';
}

function getDefaultResultOrder() {
  return dnsOrder;
}

What is the feature you are proposing to solve the problem?

Judging whether dnsOrder has changed according to the exposed api.

What alternatives have you considered?

No response

@btea btea added the feature request Issues that request new features to be added to Node.js. label Mar 3, 2023
@bnoordhuis
Copy link
Member

Can you explain why you want to know that? I'm trying to think of reasonable use cases but I'm coming up empty-handed.

To a first approximation, as a library author, you're not expected to care. That's the responsibility of the application.

As an application programmer, you normally configure it at startup and then never change nor care about it later.

@btea
Copy link
Contributor Author

btea commented Mar 4, 2023

After I start a node service, if I use getDefaultResultOrder to change the value of dnsOrder or change other contents of the same file, when hot update is triggered at this time, I hope to obtain the value before and after the change through the exposed method to judge whether the order value has changed.

According to the results of the above judgment, I will decide whether to update the local value printed on the terminal.

@bnoordhuis
Copy link
Member

Maybe I misunderstand you but that seems of really limited utility. Why would it matter if the value changed between reloads? Is it purely a diagnostic?

@btea
Copy link
Contributor Author

btea commented Mar 5, 2023

Yeah, the situation I am currently encountering is mainly to compare the values ​​before and after the change and make some changes.

However, since the logic of changing the dnsOrder value is not in the same file as the logic that requires other processing, I think it would be convenient to dynamically obtain the values ​​before and after the change for comparison.

In addition, it suddenly occurred to me that by comparing the results of dns.promises('localhost') and dns.promises('localhost', { verbatim: true }), it seems that it is possible to determine whether the value of dnsOrder has changed, but some cumbersome.

@bnoordhuis
Copy link
Member

I understand what you're saying now but I don't know if it passes the bar for inclusion in the standard library. Can I suggest you open a pull request and see how it's received?

@tniessen
Copy link
Member

tniessen commented Mar 5, 2023

I agree with Ben; I think repeatedly switching between verbatim and ipv4first at runtime and being unable to communicate this change within the application is a very niche use case.

@btea
Copy link
Contributor Author

btea commented Mar 5, 2023

Do you mean to open a pull request to expose the getDefaultVerbatim method or expose a new method getDefaultResultOrder?

@bnoordhuis
Copy link
Member

I'll leave that up to you. What you think is best.

@btea
Copy link
Contributor Author

btea commented Mar 6, 2023

Ok, I created a pr exposing the method getDefaultResultOrder. It looks like the docs also need to be updated.

nodejs-github-bot pushed a commit that referenced this issue Apr 26, 2023
PR-URL: #46973
Fixes: #46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
yjl9903 pushed a commit to yjl9903/node that referenced this issue Apr 28, 2023
PR-URL: nodejs#46973
Fixes: nodejs#46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
yjl9903 pushed a commit to yjl9903/node that referenced this issue Apr 28, 2023
PR-URL: nodejs#46973
Fixes: nodejs#46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
yjl9903 pushed a commit to yjl9903/node that referenced this issue Apr 29, 2023
PR-URL: nodejs#46973
Fixes: nodejs#46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
targos pushed a commit that referenced this issue May 2, 2023
PR-URL: #46973
Fixes: #46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
danielleadams pushed a commit that referenced this issue Jul 6, 2023
PR-URL: #46973
Fixes: #46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
MoLow pushed a commit to MoLow/node that referenced this issue Jul 6, 2023
PR-URL: nodejs#46973
Fixes: nodejs#46919
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants