fix: always return same object from client prop (#18)

* fix: Can await returned client

calling `await client` was throwing an error. The returned client should
not be thenable, and should just return the same client

* fix: always return same object from client prop

There was a bug where every time you access a client property e.g.
`client.namespace` it would return a new Proxy object, which would lead
to unexpected behaviour when accessing a client object, e.g.
`client.namespace === client.namespace` would be false. It would also
cause a lot of garbage collection slowing things down (all those Proxy
objects need to be garbage collected).

This fix caches all the sub clients that are created when you access
properties. This could be seen as a memory leak because every time you
access a client property, the value is cached, but this is effectively
lazily building the API on the server side - as long as you don't call
huge numbers of non-existent properties on the client it won't take much
more memory than the server-side API.

client.js

@@ -178,13 +178,18 @@ function createClient(channel, { timeout = 5000 } = {}) {
     emitter.removeAllListeners()
   }
 
+  const subClientCache = new Map()
+
   return createSubClient([], {})
 
   /**
    * @param {string[]} propArray
    * @param {Function | {}} target
    * @returns {ClientApi<ApiType>} */
   function createSubClient(propArray, target) {
+    const cached = subClientCache.get(JSON.stringify(propArray))
+    if (cached) return cached
+
     /** @type {ProxyHandler<any>} */
     const handler = {
       get(target, prop) {
@@ -275,6 +280,14 @@ function createClient(channel, { timeout = 5000 } = {}) {
 
     const proxy = new Proxy(target, handler)
 
+    if (propArray.length > 0) {
+      // In some ways this is a memory leak, but only if a client tries to
+      // access large numbers of methods. If the client respects the client
+      // type, then this is just lazily creating the API object referenced on
+      // the server.
+      subClientCache.set(JSON.stringify(propArray), proxy)
+    }
+
     return proxy
   }
 }

test/e2e.test.js

@@ -106,19 +106,21 @@ runTests(function setup(api, opts) {
   }
 })
 
-// Run tests with Duplex Stream
-// @ts-ignore
-runTests(function setup(api, opts) {
-  const { socket1, socket2 } = new DuplexPair({ objectMode: true })
-
-  const serverStream = socket1
-  const clientStream = socket2
-
-  return {
-    client: createClient(clientStream, opts),
-    server: createServer(api, serverStream),
-  }
-})
+if (!process.env.TAP_ONLY) {
+  // Run tests with Duplex Stream
+  // @ts-ignore
+  runTests(function setup(api, opts) {
+    const { socket1, socket2 } = new DuplexPair({ objectMode: true })
+
+    const serverStream = socket1
+    const clientStream = socket2
+
+    return {
+      client: createClient(clientStream, opts),
+      server: createServer(api, serverStream),
+    }
+  })
+}
 
 /**
  * @typedef {<T extends {}>(api: T, opts?: Parameters<typeof createClient>[1]) => { client: ClientApi<T>, server: ReturnType<typeof createServer>}} SetupFunction
@@ -551,10 +553,21 @@ function runTests(setup) {
     t.end()
   })
 
-  test('Can await client', async (t) => {
+  test('client properties are stable', (t) => {
+    // If we don't cache the proxy returned by accessing a property like
+    // `client.namespace`, then each time you access it a new Proxy will be
+    // created.
+    const { client } = setup(myApi)
+    t.is(client.namespace, client.namespace)
+    t.is(client.deep.nested, client.deep.nested)
+    t.end()
+  })
+
+  test('Can await client and subclients', async (t) => {
     const { client } = setup(myApi)
-    const awaited = await client
-    t.is(awaited, client, 'Same object is returned when awaiting')
+    t.is(await client, client, 'Same object is returned when awaiting')
+    t.is(await client.deep, client.deep)
+    t.is(await client.deep.nested, client.deep.nested)
     t.end()
   })
 }