search

jakeburden / next-absolute-url

Get the absolute URL of your Next.js app (optionally set a localhost dev URL)
300 stars 30 forks source link

Rewrite to Typescript #8

Closed jerrygreen closed 4 years ago

jerrygreen commented 4 years ago

next-absolute-url doesn't have Typescript definitions:

Screenshot 2020-01-13 at 18 47 20

What this PR does:

  1. Rewrite to Typescript. It has index.ts now, which is actual source code now, and by running yarn build it will generate index.js (which stays to be pretty much the same as earlier) and index.d.ts with type definitions

  2. Remove req.headers['x-forwarded-host'] as host source. I don't receive this header myself, and also I haven't found it at node type definitions: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/http.d.ts

  3. Use /^localhost(:\d+)?$/.test(host) regular expression instead of host.indexOf('localhost') > -1. No very much sense here but urls like fancy-localhost.com won't be treated as actual localhost now

jerrygreen commented 4 years ago
  1. Would you recommend a version bump for a typescript rewrite?

Yea. I'll make a version bump commit right away

  1. Would you be willing to add a test for these changes?

Yea, I'll now think about this too

jerrygreen commented 4 years ago

@jekrb I have a question actually, - why you've initially added host.indexOf('localhost') > -1, does it have any sense, really?

UPD. Uh, wait, I'm silly, that's the initial sense of the lib 😅 so we can define protocol properly

jerrygreen commented 4 years ago

@jekrb I think that's all. Have a look?

jakeburden commented 4 years ago

@JerryGreen this looks really good to me. Thank you so much for writing the tests! Merging now. 🚀

jakeburden commented 4 years ago

@JerryGreen this has been publish on npm next-absolute-url@1.2.0 🎉