Skip to content

feat: fetch proxy #411

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ojeytonwilliams
Copy link
Contributor

Checklist:

It's a bare-bones implementation, but it should support the functions we currently use and let devs know when they try to use something that's yet to be implemented

@ojeytonwilliams ojeytonwilliams requested a review from a team as a code owner July 18, 2025 17:15
@ojeytonwilliams ojeytonwilliams marked this pull request as draft July 21, 2025 07:14
The client should not need to know about how fetch is handled - that's
the responsibility of the runner.
@ShaunSHamilton
Copy link
Member

Is this just useful for spying on the stuff?

@ojeytonwilliams
Copy link
Contributor Author

Mostly it's so we can fix this: freeCodeCamp/freeCodeCamp#60874

@ShaunSHamilton
Copy link
Member

Last question: Do we want to default to replacing fetch? Or, instead just offer a fetchProxy?

@ojeytonwilliams
Copy link
Contributor Author

That's a good question... and one I should have thought about.

I'm leaning towards replacing fetch by default just because it's less work on both sides. Partly because it's straightforward, partly because I can't think of a reason not to (other than the unimplemented functionality), but mostly because it's reversible. Nothing gets locked in if we choose the path of least resistance.

What do you reckon?

@ShaunSHamilton
Copy link
Member

ShaunSHamilton commented Aug 22, 2025

Good point. If it works, worst case scenario is eventually creating a challenge (test) that needs a change.

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending merge conflict 👍

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.

2 participants