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

Fetch+ #1492

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Fetch+ #1492

wants to merge 14 commits into from

Conversation

PPPDUD
Copy link

@PPPDUD PPPDUD commented May 28, 2024

Fetch+ is like Fetch, but it has support for a CORS proxy too.

@LilyMakesThings
Copy link
Contributor

The problem with having CORS proxies "built in" to extensions like this is that, the nature of the internet and APIs is that they tend to go offline after an amount of time. We need them to live for as long as TurboWarp theoretically will, and the more we depend on external websites like this, the more projects that might break in the future (projects by real people).

One example is "S-Grab", which is now fundamentally pointless because the API that it relied on appears to be down or dysfunctional.

Providing some kind of direct and more explicit way to tell people about CORS errors (or fetching errors in general) would encourage them to use google and find a CORS proxy of their own - that way if it goes down, TurboWarp isn't to blame.

@PPPDUD
Copy link
Author

PPPDUD commented May 28, 2024 via email

@AshimeeAlt
Copy link
Contributor

AshimeeAlt commented May 28, 2024

As always, if a CORS proxy goes down, a fellow TurboWarp user can always swap it out in the source code. Plus, it's increasingly hard to find a good CORS proxy that's not paid these days.

--- Even if CORS errors did show up, most users would not know that they need to use a proxy, let alone which one. In my extension, even if the default proxy doesn't work, you can always use a fallback proxy with the (fetch [x]) block.

we should not have to manually update the proxy every time it goes down or stops working,
aside from this your pr failed all checks, and goes against many practices, along with fetch
and similar extensions to this already existing, which also goes against a guideline, a dedicated
extension to just fetch a URL with another service is pointless, as the user themselves should be the one
finding and choosing the proxy, not the extension itself.

@PPPDUD
Copy link
Author

PPPDUD commented May 28, 2024

As always, if a CORS proxy goes down, a fellow TurboWarp user can always swap it out in the source code. Plus, it's increasingly hard to find a good CORS proxy that's not paid these days.

--- Even if CORS errors did show up, most users would not know that they need to use a proxy, let alone which one. In my extension, even if the default proxy doesn't work, you can always use a fallback proxy with the (fetch [x]) block.
On Mon, May 27, 2024, 8:35 PM LilyMakesThings @.> wrote: The problem with having CORS proxies "built in" to extensions like this is that, the nature of the internet and APIs is that they tend to go offline after an amount of time. We need them to live for as long as TurboWarp theoretically will, and the more we depend on external websites like this, the more projects that might break in the future (projects by real people). One example is "S-Grab", which is now fundamentally pointless because the API that it relied on appears to be down or dysfunctional. Providing some kind of direct and more explicit way to tell people about CORS errors (or fetching errors in general) would encourage them to use google and find a CORS proxy of their own - that way if it goes down, TurboWarp isn't to blame. — Reply to this email directly, view it on GitHub <#1492 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZTWPZKAVJTMWNWPFE4TRRTZEPGNTAVCNFSM6AAAAABIL4DN32VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZUGE3TINBRHE . You are receiving this because you authored the thread.Message ID: _@**.**_>

we should not have to manually update the proxy every time it goes down or stops working,

P.S: your pr failed all checks

I have a problem where I start flame wars easily, so (no offense) I need to stop replying to criticism for a bit.

@AshimeeAlt
Copy link
Contributor

AshimeeAlt commented May 28, 2024

As always, if a CORS proxy goes down, a fellow TurboWarp user can always swap it out in the source code. Plus, it's increasingly hard to find a good CORS proxy that's not paid these days.

--- Even if CORS errors did show up, most users would not know that they need to use a proxy, let alone which one. In my extension, even if the default proxy doesn't work, you can always use a fallback proxy with the (fetch [x]) block.

I have a problem where I start flame wars easily, so (no offense) I need to stop replying to criticism for a bit.

understandable

@LilyMakesThings
Copy link
Contributor

As always, if a CORS proxy goes down, a fellow TurboWarp user can always
swap it out in the source code.

There are over 100 extensions in this repository that have only 1 consistent maintainer. Reducing the amount of maintenance on each extension is an absolute must, and submitting an entire extension that can only work if it is maintained consistently is simply an unreasonable expectation.

At least if a user of TurboWarp runs into an issue with the CORS proxy they're using in their own code, they can just change it themselves. But what happens when their code suddenly breaks for no apparent reason? Bug reports.

I don't know, this extension just seems like a fundamental nightmare.

@PPPDUD
Copy link
Author

PPPDUD commented May 29, 2024 via email

@PPPDUD PPPDUD marked this pull request as ready for review May 29, 2024 15:09
@LilyMakesThings
Copy link
Contributor

LilyMakesThings commented May 29, 2024

Here's my hopefully final argument: the CORS proxy is for convenience, not resilience.

Then it's not fit for here, I don't know what to tell you

@GarboMuffin
Copy link
Member

We do need to properly educate people about CORS and why their fetch blocks usually don't work, that much is true

@Xeltalliv
Copy link
Contributor

In addition to concerns listed above, there is another one: a lot of extensions have some kind of "load the thing from URL" blocks, but your current solution only addresses the issue for 1 specific extension.

@jabinstech
Copy link

corsproxy.io is paid, the free version only works up to a certain number of requests and then doesn't let you make any after that

nuh uh

@mybearworld
Copy link
Contributor

Yes, sorry, that was incorrect. It requires you to create an account at a certain request point, though.

@NotFenixio
Copy link

According to the contribution guide, shouldn't this just be an update to the original Fetch extension? The normal fetch mode on
this extension is just like the original one but with more boilerplaté and the CORS proxy mode is a copypaste but with a string
concatenation slapped in the url field.

@AshimeeAlt
Copy link
Contributor

According to the contribution guide, shouldn't this just be an update to the original Fetch extension? The normal fetch mode on this extension is just like the original one but with more boilerplaté and the CORS proxy mode is a copypaste but with a string concatenation slapped in the url field.

yes it technically should be an update, but...
aside from that the extension is unnecessary so is the modification, to quote garbo:

We do need to properly educate people about CORS and why their fetch blocks usually don't work, that much is true

which does not mean just slapping corsproxy as an option, plus what they said above.

@SharkPool-SP
Copy link
Collaborator

According to the contribution guide, shouldn't this just be an update to the original Fetch extension? The normal fetch mode on this extension is just like the original one but with more boilerplaté and the CORS proxy mode is a copypaste but with a string concatenation slapped in the url field.

yes it technically should be an update, but...

aside from that the extension is unnecessary so is the modification, to quote garbo:

We do need to properly educate people about CORS and why their fetch blocks usually don't work, that much is true

which does not mean just slapping corsproxy as an option, plus what they said above.

It's like making an extension that takes the join block, and making it join a string and "!" to it

@FurryR
Copy link
Contributor

FurryR commented Jul 3, 2024

This extension shouldn't be merged.

  1. Fetch+ is not suitable for the extension. When we see Fetch+ we are thinking about fetching streams, submitting forms and more, while the extension is just fetch with a CORS proxy.
  2. corsproxy.io may monitor and record the requests between the user and the server. It may cause passwords or private information to leak.
  3. corsproxy.io forbids access from some regions including China mainland and Hong Kong, and the extension didn't provide a method to change CORS proxy provider.

@namelessisbackbutnottodobadthingsiswear

This extension shouldn't be merged.

I feel like there's still a chance for this extension, despite it's many flaws. For reference, I made a similar PR a while ago (#760) and although it never got merged, some of the fundamental ideas people have brought up in this conversation do carry over from it.

  • The extension would need to be more clear about it's use in the title. I agree "Fetch+" sounds a little misleading, and as this extension should act as a tutorial for CORS proxy, having it in the title would help.
  • Adding documentation to explain the purpose of CORS proxies and how to use them without the extension would be a necessity. Considering we cannot count on external APIs to be up all the time, and the end goal is to teach people why they need a CORS proxy, having some form of textual explanation to link to would help provide the extension more purpose.
  • Perhaps an option to swap CORS proxy providers would be in order. Of course, there's still the chance all are down at once, however it's exponentially smaller, and would be less work to maintain in the event one does permanently shut down.

@LilyMakesThings
Copy link
Contributor

A more appropriate solution would be to have a better error response for Scratch.fetch from a CORS error. An extension entirely dedicated to just being a CORS proxy is not a solution when it is a fundamentally flawed concept on a number of levels; allowing users to educate themselves on CORS proxies, however...

@RockyTheProtogen
Copy link

Just gave it a shot. And I think there are tons of valid points.
But the main one don't hard-code the proxy. You might like a default, but don't lock it in place.
Adding an argument might be a better option.
Also, change the name to something like CORS Fetch which would be less misleading.

@GarboMuffin GarboMuffin added the pr: new extension Pull requests that add a new extension label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: new extension Pull requests that add a new extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.