Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use native fetch, remove request, update minimist #509

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isaacs
Copy link

@isaacs isaacs commented Aug 30, 2023

This fixes all of the production audit warnings. There are still some dev dependency advisories, due to the old version of mocha which has old pinned deps.

BREAKING CHANGES:

  • native fetch requires node v16.15 or higher.

Note: this is just a first pass to hopefully start a conversation about how to modernize this a bit. I tried to make it as surgical as possible, but a full modernizing workup with ESM and all the bells and whistles would be another option.

Quite a few tests are failing on my system with this change, so it's probably not ready to land as-is. But, many tests are failing on my system without this patch, so it's a bit hard to know what is a new failure.

This fixes all of the production audit warnings. There are still some
dev dependency advisories, due to the old version of mocha which has
old pinned deps.

BREAKING CHANGES:

- native fetch requires node v16.15 or higher.
@sintaxi
Copy link
Owner

sintaxi commented Sep 1, 2023

Isaac! 🔥🔥🔥 This is amazing!

I started the undertaking a couple years ago and never got it entirely over the finish line but got very close.

Here is what I did....

  • I moved all business logic to a library called surge-sdk
  • the surge "list-rollback" branch is now just the CLI "views" and it uses the surge-sdk
  • all API call in surge-sdk have been switched from using "request" to "axios" except...
  • surge-stream is the part that uploads the zip to the API (this still uses request)
  • surge-stream "master" has an attempt to port over request to axios (not all tests are passing).
  • code in surge, surge-sdk, and surge-stream is quite clean. dependencies got minimized.

The most up to date branch of surge (by far) is list-rollback and it actually has a lot of awesome shit in it. If you want to kick the tires I suggest installing [email protected] I have cut 15 release candidates from the list-rollback branch with rc.14 being the most stable.

I should probably begin by merginglist-rollback because it is 131 ahead of master. Then we can see what can be done about getting your work merged into surge-stream.

Thanks for the PR. It made my day. Hope you are well!

@isaacs
Copy link
Author

isaacs commented Sep 3, 2023

Oh, sweet, I didn't realize there's a branch with more work done on it, but that does make sense. I'll try to see if I can port it onto that branch.

I've switched a few libs from request to fetch by this point, so the change was pretty mechanical. I honestly don't know what most of those commands were doing, just swapped the APIs 😅

And yeah, I was surprised to see your name too, didn't realize surge was your thing until I went to send this PR. It's been a minute, hope things are going well in your world ;)

I did end up still using netlify for node-tap's prod website, even though surge deployment is a way simpler, just because I can't be assed to manage TLS certs. Is there any plan to have something like that for surge.sh?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants