Skip to content
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

fix: always return same object from client prop #18

Merged
merged 3 commits into from
Sep 18, 2023

Conversation

gmaclennan
Copy link
Member

Stacked on #17

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.

calling `await client` was throwing an error. The returned client should
not be thenable, and should just return the same client
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.
@gmaclennan gmaclennan self-assigned this Sep 15, 2023
Base automatically changed from fix/awaited-client to main September 18, 2023 17:12
@gmaclennan gmaclennan merged commit 8517f2f into main Sep 18, 2023
3 checks passed
@gmaclennan gmaclennan deleted the fix/stable-property-access branch September 18, 2023 22:44
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.

2 participants