fixed some error and added one new feature #19
Conversation
hello, I have just fixed an error, which occurred when i used this module with local ip(192.168.0.1 etc..). It retured https protocols insted of http. i added test as well. Furthermore i have added one more argument for function, which is options. If user wnats to customized https or http manually then s/he can pass custom https:true. Also I have written documentation for usage of new feature on mark down. if you find any difficulties to understand changes, sont hasited to ask me.
| let protocol = /^localhost(:\d+)?$/.test(host) ? 'http:' : 'https:' | ||
| let protocol = https | ||
| ? 'https:' // if NODE_ENV is production | ||
| : process.env.NODE_ENV == 'production' |
There was a problem hiding this comment.
Question:
This seems to lock the protocol to https if process.env.NODE_ENV == 'production'.
In most cases I think this will be fine, but what if someone is testing their production build locally, over http?
I'm not sure if we should be changing things based on environment variables here. Thoughts?
|
Thank you so much @manan5439 for your contribution and for including tests! I left one question, would love to get your thoughts on it. Back to you 🏓 |
|
Hi @jekrb! Could you possibly merge this fix and remove the https defaulting for production for the reasons you stated? My fetch calls are breaking 😓 |
|
@areksds have you tested this change to see if it fixes the problem? I think you can do I haven't had time to test this myself (I need to lean on community contributions at the moment 🙇), I'd feel much more comfortable merging this change sooner if you can confirm that it works for you. |
|
@jekrb just tested on localhost and production. It does make all localhost requests (127.0.0.1, etc.) http by default which is great, but since production is forced as https that only fixes half of the problem 🙃 Is there any way to just check what the incoming request's protocol is and use that as the protocol instead of setting it with a regex / checking whether it's a production build? Thanks for your time! |
@areksds just out of curiosity, are all your environments If so, a quick fix could be: const { host } = absoluteUrl(req)
const apiURL = `http://${host}/api/your_endpoint.js`At least until we resolve all the issues with this module. |
hello,
I have just fixed an error, which occurred when i used this module with local ip(192.168.0.1 etc..). It returned https protocols insted of http. i added test as well.
Furthermore i have added one more argument for function, which is options. If user wants to customised https or http manually then s/he can pass custom https:true. Also I have written documentation for usage of new feature on mark down.
if you find any difficulties to understand changes, don't haste to ask me.