Handle ngrok version 3 #272
Conversation
|
looks great to me! thanks again <3 |
|
Hey guys 👋🏻, do we know when this is going to get a merge? 👀 |
|
@philnash, do you see any concerns merging it? LGTM |
|
Really looking forward to seeing this merged! |
|
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. |
|
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. |
This commit refactors a couple of things too. Mostly it extracts things from the src/process.js file. Neither the version or authtoken commands need the process (they both spawn their own processes briefly), so they had no business colocating. This allowed the setAuthToken function to break out into a few utility functions and make the function more understandable overall. The new src/version.js didn't change much from when it was in src/process.js, but relocating it did allow for removing some requires at the top of src/process.js
|
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. |
|
Thanks @philnash ! anything we can do to move this forward? |
|
Hi, PM from ngrok here. Really looking to get this PR moving. Let me know if there's anything I can help with. |
|
@philnash any updates on when this is getting merged? |
|
@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? |
|
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. |
I think this is all that is needed to handle ngrok 3 (#271). It updates the
authtokenfunction to handle making a different command based on whether the underlying ngrok executable is version 2 or version 3. It also adds anupgradeConfigmethod to upgrade ngrok config so that the new version can read it.