-
Notifications
You must be signed in to change notification settings - Fork 31.3k
http: support FetchEvent API #49405
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
base: main
Are you sure you want to change the base?
http: support FetchEvent API #49405
Conversation
Review requested:
|
This is something I thought about doing years ago but I never moved forward with it because:
I thought WinterCG had this as a recommendation (that would be a good argument pro) but seems I misremember. |
It does enable some cross-compatibility with modules targeting workers style runtimes like Cloudflare though, which I think is valuable. I also think orienting around standards-based types is a good thing long-term. I think it's at least worth the experiment to figure out if the standard APIs are suitable for the need in the event we would want to consider a more standards-oriented approach in the future. Certainly it being based on WHATWG streams and EventTarget though the perf is likely not great at the moment. If we want to land this at all I'd say probably we should have it behind a flag until we're sure of the value of supporting it. |
What's performance like? |
I'm not exactly convinced by adding this to |
Yeah, I'm not entirely convinced myself. I like the idea of having something more standards-based purely from an alignment perspective, but I'm not entirely sure if it's the right time given the perf implications of web streams and EventTarget. I do also agree with @bnoordhuis that having Yet Another Way to do the same thing may not be a great idea until we're a bit more certain of it. This is an experiment I felt like trying out while on vacation. I opened the PR mainly to start the conversation around what aligning with web standards means when it comes to http servers and if there's a direction we should be working towards in some way. It seems like many have gone the direction of implementing a ServiceWorker-like thing, which we could do, but perf implications are definitely important to consider in deciding if it's something we actually want to do or when it may make sense to do such a thing. What do we feel should block this? Certainly perf of web streams should be improved. Are there http-server-specific considerations we need to identify? Is the interface reasonable from a usability perspective or are there tweaks we should be considering? Are there things we could take to standards proposals now to adapt what we have to something more usable for us? |
I don't disagree. I initially wanted to suggest proposing it for standardization to WinterCG but it's probably too opinionated. Deno Deploy started out with the ServiceWorker model but later switched to something more flexible (source: me - employee at the time.) |
I think we could specify it a bit more loosely than needing to be any sort of worker specifically. We could just say there's a fetch event on something which conforms to the FetchEvent spec and surrounding types, then the specific implementation of where that fetch event exists can vary. That also leaves it open to do what I mentioned in the initial post of also using it for actual ServiceWorker-style intercepting of outgoing HTTP. The question then is: are those existing standard types sufficient for what we want to do with them? Can they be expanded to enable server runtime use cases? For example, there are known issues with web streams that make them not work well for server use when built to-spec, but we could propose some sort of strategy switching system with sensible defaults depending on the environment to make them behave in a more reasonable way for the constraints of our specific environment. |
This is an experiment to see if it would be reasonable to support the FetchEvent interface on an http server. My thinking was to start with a named event directly on the server for the moment and then extend that with more ServiceWorker style thread isolation as a layer on top of that later. Could also consider a separate
createFetchServer
factory or modifying the existingcreateServer
to mode switch by function parameter count. Just wanted to share my little MVP first though to see what people think and discuss direction or if we want to pursue this at all. 🤔Note that this is still very much an experiment so probably lots of bugs and usability issues. Also no docs yet, you can see the test for an example of how it works. Just want to get feedback before I put more work into it.
I was also thinking of pulling out the Handle Fetch part of the spec to something that could be used both for processing incoming HTTP requests and also put it on
http.Agent
to allow ServiceWorker style request mocking or caching. Could be a useful tool.cc @nodejs/http @nodejs/undici @nodejs/whatwg-stream