Skip to content

Conversation

@Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Jan 4, 2023

Packages sometimes call Socket() instead of new Socket

Is this a suitable workaround without converting to prototype object/non-class? @alexlamsl

@Jarred-Sumner
Copy link
Collaborator Author

This doesn't work for Socket.prototype.constructor === Socket checks, which makes sense

hm

@alexlamsl
Copy link
Contributor

This should work:

--- a/src/bun.js/net.exports.js
+++ b/src/bun.js/net.exports.js
@@ -57,7 +57,17 @@ const { Bun, createFIFO } = import.meta.primordials;
 const { connect: bunConnect } = Bun;
 const { Duplex } = import.meta.require("node:stream");
 
-export class Socket extends Duplex {
+export const Socket = function(InternalSocket) {
+  return Object.defineProperty(function Socket(options) {
+    return Object.defineProperty(new InternalSocket(options), "constructor", {
+      value: Socket,
+    });
+  }, Symbol.hasInstance, {
+    value(instance) {
+      return instance instanceof InternalSocket;
+    },
+  });
+}(class Socket extends Duplex {
   static #Handlers = {
     close: Socket.#Close,
     connectError(socket, error) {
@@ -286,7 +296,7 @@ export class Socket extends Duplex {
       this.#writeChunk = chunk;
     }
   }
-}
+});
 
 export function createConnection(port, host, connectListener) {
   if (typeof host == "function") {

@Jarred-Sumner Jarred-Sumner marked this pull request as ready for review January 4, 2023 12:06
@Jarred-Sumner Jarred-Sumner merged commit e2231f1 into main Jan 4, 2023
@Jarred-Sumner Jarred-Sumner deleted the jarred/make-postgres.js-work branch January 4, 2023 12:06
Jarred-Sumner added a commit that referenced this pull request Jan 6, 2023
* Support non-classes

* Update net.exports.js

* Make it less observable

* Update net.exports.js

Co-authored-by: Jarred Sumner <[email protected]>
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.

3 participants