deno
deno copied to clipboard
fix(ext/node): refactor http.ServerResponse into function class
Fixes #19901
Context
light-my-request is used internally by fastify.inject(), which in turn is used to test routes without spinning up an HTTP server.
Since it relied on http.ServerResponse.call() it could not work on Deno, which used an ES6 class.
This PR should fix that.
Technical
While testing, I found out that light-my-request relies on ServerResponse.connection, which is deprecated, so I added that and socket, the non deprecated property.
It also relies on an undocumented _header property, apparently for raw header processing.
I added it as an empty string, feel free to provide other approaches.
Code is definitely uglier now, but there are not many options... suggestions are welcome!
@nicolabovolato thanks for the PR. Could you please add a test to tests/unit_node/http_test.ts that tests new APIs you added as well as creating a ServerResponse using "ES5 style classes"?
@nicolabovolato thanks for the PR. Could you please add a test to tests/unit_node/http_test.ts that tests new APIs you added as well as creating a ServerResponse using "ES5 style classes"?
@bartlomieju I added some, let me know if we should have more 👍🏼
@bartlomieju I think I've found abetter way to do this, without es5 classes and without rewriting the whole ServerResponse class.
import { EventEmitter } from "node:events";
import { EventEmitterOptions } from "npm:@types/node";
class Base extends EventEmitter {
constructor(options?: EventEmitterOptions) {
super(options);
}
static override call(cls: any, options?: EventEmitterOptions) {
const x = new this(options);
Object.assign(cls, x);
}
}
function Class(this: any) {
Base.call(this, {});
}
Object.setPrototypeOf(Class.prototype, Base.prototype);
const x = new (Class as any)();
console.log(x);
x.on("foo", (data: any) => console.log(data));
x.emit("foo", "bar");
I'll open another pr in the following days.
@nicolabovolato I think the current solution is fine, it's closer to what Node.js is doing, so I'd prefer to go with this approach.
@littledivy please take a look - this is fine for me so if you don't have any objections we should land it.