Skip to content

Sketch of new Request(Proxy(Request))#6730

Draft
jasnell wants to merge 1 commit into
mainfrom
jasnell/request-proxy-sketch
Draft

Sketch of new Request(Proxy(Request))#6730
jasnell wants to merge 1 commit into
mainfrom
jasnell/request-proxy-sketch

Conversation

@jasnell
Copy link
Copy Markdown
Collaborator

@jasnell jasnell commented May 6, 2026

A skech of an approach to addressing #6700

This is not quite functional yet due to a limitation with handling jsg::Objects through the lens of JSG_STRUCT... well, more limitation of handling jsg::Objects through Proxy:

  const rp = new Proxy(r, {
    get(...args) {
      return Reflect.get(...args);
    }
  });

  console.log(rp.url);  // throws

This is a different problem tho. The sketch shows how we can make new Request(Proxy(Request)) work in general.

ask-bonk[bot]

This comment was marked as outdated.

@ask-bonk

This comment was marked as outdated.

@codecov-commenter

This comment was marked as outdated.

@vicb
Copy link
Copy Markdown
Contributor

vicb commented May 6, 2026

@jasnell while trying to validate the fix, I noticed that the lockfile is out of date. I created #6731

However doing so breaks the build.
But the build also fails with the checkedin lockfile on mac.

@jasnell
Copy link
Copy Markdown
Collaborator Author

jasnell commented May 6, 2026

@vicb ... this won't work anyway until the more general issue of wrapping jsg::Object's with proxies is addressed.

Comment thread src/workerd/api/http.c++
fetcher = oldRequest->getFetcher();
signal = oldRequest->getSignal();
}
KJ_CASE_ONEOF(p, jsg::Proxy<RequestProxy>) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of reproducing all this logic here, it would be nice if we could delegate to the existing RequestInit path. Only the url needs special handling.

Otherwise it's easy to forget to add new properties here.

Comment thread src/workerd/api/http.h
void validate(jsg::Lock&);
};

struct RequestProxy {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we possibly use RequestInit here instead of declaring a new type? It seems ugly that we'd have to extend this type later.

I guess the only problem is that RequestInit doesn't include url. Though maybe it could -- just declare it optional, and ignore it except in the proxy case?

Relately, I see you dismissed Bonk's comment pointing out that cache and encodeResponseBody are missing, but that does seem like an actual problem, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Dismissed to reduce a small amount of noise in the discussion. Already planning to account for both of we decided that this was a worthwhile approach.

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.

4 participants