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

feat: replace the 'fast-url-parser' module with the 'node:url' module #207

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

gweesin
Copy link
Contributor

@gweesin gweesin commented Apr 4, 2024

closed #204

@jimthedev
Copy link

jimthedev commented Oct 15, 2024

@gweesin You probably already know this but I think @leo was previously merging on this repo but has left Vercel (if his twitter is any indication). Not so sure this is getting merged unless someone else from over there has the ability to approve. Maybe @rauchg or @TooTallNate knows who hits the buttons on this repo now?

@leo
Copy link
Contributor

leo commented Oct 15, 2024

I indeed don't have access to this repo anymore, but I just pressed "View git blame" on the line that imports fast-url-parser, which took me to this commit. That commit replaces the native url module with fast-url-parser.

I was wondering why (I don't remember the reason), but looking at the readme of the fast-url-parser package and considering it has 4 million weekly downloads, it seems that the only reason why the package exists is to make the native url module 25x faster (e.g. for parse()). That's why it has the same API too.

So unless the native url module got a lot faster over the years (which is probably the case, but we'd have to test to what extend that is the case), using it might make serve-handler a lot slower in production.

But agreed that, if the owner of fast-url-parser is not responsive, replacing it in server-handler might be the only option. I think it is very likely that the native url module has gotten faster over the years.

@regseb
Copy link

regseb commented Oct 15, 2024

I've run fast-url-parser benchmarks (higher is better) with Node v22: fast-url-parser is 2.5 times faster than node:url.

October 15, 2024 / Node 22.9.0 fast-url-parser node:url Diff
parse() 509333.95 216544.43 2.35
format() 567429.07 187700.54 3.02
resolve("../foo/bar?baz=boom") 579789.47 193241.46 3.00
resolve("foo/bar") 579329.57 194313.53 2.98
resolve("http://nodejs.org") 605089.97 190518.45 3.18
resolve("./foo/bar?baz") 607911.60 187227.32 3.25

For your information, here is the benchmark displayed in fast-url-parser's README dating back 11 years. fast-url-parser was 10 times faster.

December 29, 2013 / Maybe Node 0.10.x fast-url-parser node:url Diff
parse() 509333.95 16459 24.4
format() 567429.07 15978 15.9
resolve("../foo/bar?baz=boom") 56701.419 6837.7 8.29
resolve("foo/bar") 80059.500 7038.6 11.4
resolve("http://nodejs.org") 118566.13 6491.1 18.3
resolve("./foo/bar?baz") 62778.648 6968.4 9.01

@leo
Copy link
Contributor

leo commented Oct 15, 2024

That seems like a reasonable tradeoff to me, considering that using the native module would also make the package lighter.

Great to see that Node has gotten a lot faster. I'll try to ping someone at Vercel to merge this PR.

@AndyBitz
Copy link
Contributor

Thanks for the PR @gweesin and thanks for the benchmarks @regseb!

I'll try to cut a release later today with those changes

@AndyBitz AndyBitz merged commit 3c3854b into vercel:main Oct 15, 2024
1 check passed
@jimthedev
Copy link

Thank you all for hopping on this. Really appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module punycode is deprecated since Node.js 21
5 participants