search

tediousjs / tedious

Node TDS module for connecting to SQL Server databases.
http://tediousjs.github.io/tedious/
MIT License
1.58k stars 439 forks source link

fix: use userland module for `punycode` #1596

Closed EliTheCoder closed 10 months ago

EliTheCoder commented 10 months ago

The slash at the end is necessary to use the userland module. The module built-in to node is deprecated.

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (57998f2) 79.00% compared to head (d6fb8c9) 78.07%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1596 +/- ## ========================================== - Coverage 79.00% 78.07% -0.94% ========================================== Files 93 93 Lines 4821 4821 Branches 921 921 ========================================== - Hits 3809 3764 -45 - Misses 709 756 +47 + Partials 303 301 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MichaelSun90 commented 10 months ago

Hi @EliTheCoder, Thanks for raising this PR. I also tried to use the url.domainToASCII function to replace the one that used to be provided by punnyCode. Seems the related tests still behave the same way. Not sure whether we want still keep the punnyCode or just use url instead. Hi @arthurschreiber, seems url.domainToASCII can handle what punycode.toASCII used to do. Do we want still keep punnyCode or just go with the url lib?

MichaelSun90 commented 10 months ago

Just double checked that Node 14 seems do not support node:url but we should be safe since the minimum node version maintained on tedious side is node 16 :thinking: .

arthurschreiber commented 10 months ago

Thanks for your contribution! ❤️

github-actions[bot] commented 10 months ago

:tada: This PR is included in version 16.7.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: