digitalocean-js icon indicating copy to clipboard operation
digitalocean-js copied to clipboard

Cleanup unnecessary nested promises in services.

Open tracker1 opened this issue 3 years ago • 2 comments

I created a fork of this repo, in order to use it with Deno at tracker1/digitalocean-deno.

One of the things I did, is replace axios with fetch, the other is I cleaned up the services in order to remove the unnecessary nested promises. If you were interested adapting back for use in this repo should be relatively easy... mostly, copying the lib folder, and from there replacing the include... from '*.ts' removing the .ts file suffix. I also renamed the polyfill I made with fetch to request, so I had to rename some of the request data variables to data.

If you're open to merging these changes back in, I can create a pull request for you... the changes to fetch aren't strictly necessary, and could keep axios (changing the instance export name, and keeping the setup), alternatively could use the node-fetch module, and remove it entirely for browsers, and for node after 18.x becomes the LTS release, if you wanted to.

I'm hoping to get it configured for a publish to Denoland ... and possibly getting a browser and node build path... I notice there's only one test file in place, so will try and port those to deno test syntax. If you're at all interested in working together, I'm happy to do so... It seems you know the DO API far better than I do... I mostly just cleaned, removed some extra bits, and minor cleanup.

tracker1 avatar Jul 09 '22 00:07 tracker1

Hey @tracker1 sorry for the delay in going through this, it's been a busy few months for me. Love to see your desire to use this project in Deno! I haven't had a chance to look through your changes, so give me a little time to go through it and see whether I'm interested in merging those changes back in. I'm in the process of migrating my whole library setup to use NX instead of the current setup that is using a lot of old technologies, so that's going to move some stuff around. Nothing majorly changed to the code itself, but it may be worth waiting until that change is merged.

Again, love the enthusiasm for the project! I'll get back to you as soon as I can!

johnbwoodruff avatar Jul 18 '22 19:07 johnbwoodruff

@johnbwoodruff understandable... also have a todo list a mile long for myself on top of the day job. Will keep an eye out to more replies in this thread.

For now, I was able to get the library working for my immediate needs, which was the main thing at least for my own use case. Kind of wild how much the JS/TS ecosystem has shifted over time... It's almost another dark ages trying to keep what goes into the browser, deno, node etc all working in something resembling reusable code. I'm really liking Deno a lot more than Node in so many ways, but the rich npm packages is something that's not the easiest thing to get over, and some of the bridge options aren't great for modern(ish) packages.

tracker1 avatar Jul 24 '22 22:07 tracker1

@johnbwoodruff ping

tracker1 avatar Sep 15 '22 00:09 tracker1

Looping back on this, I finally completed a fairly major overhaul to the project structure to convert it to an NX workspace instead of the custom typescript seed project I originally used. I've also begun the process of writing some tests for the library. As part of that project overhaul, since I was changing everything anyway, I also went through and unwrapped the promises like you requested. Great suggestion, thanks! Closed in #49.

johnbwoodruff avatar Oct 23 '22 06:10 johnbwoodruff