search

bubenshchykov / ngrok

Expose your localhost to the web. Node wrapper for ngrok.
https://ngrok.com
2.33k stars 317 forks source link

Handle ngrok version 3 #272

Closed philnash closed 1 year ago

philnash commented 2 years ago

I think this is all that is needed to handle ngrok 3 (#271). It updates the authtoken function to handle making a different command based on whether the underlying ngrok executable is version 2 or version 3. It also adds an upgradeConfig method to upgrade ngrok config so that the new version can read it.

bubenshchykov commented 2 years ago

looks great to me! thanks again <3

viralpickaxe commented 2 years ago

Hey guys 👋🏻, do we know when this is going to get a merge? 👀

bubenshchykov commented 2 years ago

@philnash, do you see any concerns merging it? LGTM

smithki commented 2 years ago

Really looking forward to seeing this merged!

philnash commented 2 years ago

Apologies, I’ve been travelling bunch recently and not able to spend time with this. I’ve no concerns merging, and I can do next week. But if @bubenshchykov wants to and make a release I’m perfectly happy with that too.

philnash commented 2 years ago

So, I think the download of ngrok version 3 is a different URL to the download of version 2. This means that this change is not required under version 2, and indeed we can take a step back, update the download URLs and make a major version change in this library to bring in ngrok 3.

philnash commented 2 years ago

I think this is ready for an actual review, with an eye on merging and releasing version 5 of this library!

Note, I still haven't used any of the new ngrok features (OAuth, etc) so I don't know how that works yet, or whether this library fully supports using that. That's something to find out though, as we release the new version and find out who wants to use those features.

russorat commented 1 year ago

Thanks @philnash ! anything we can do to move this forward?

russorat commented 1 year ago

Hi, PM from ngrok here. Really looking to get this PR moving. Let me know if there's anything I can help with.

meetkoriya13 commented 1 year ago

@philnash any updates on when this is getting merged?

bubenshchykov commented 1 year ago

@philnash, PR looks good, I merged it, super sorry if I caused a delay.

I don't have a laptop at the moment to fix a small test failure (one of ngrok error texts changed) and to bump the version on npm. Could you please do it?

philnash commented 1 year ago

Hey @bubenshchykov! I just hadn't got around to this too, wasn't necessarily blocked on you.

I will try to find some time this week to fix the failure and push out a beta/rc version. I'd really like to test this out in my VSCode extension to make sure it works ok and to hopefully write up any migration requirements too.