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

Have client.connect() return a Promise<client> instead of Promise<void> #2601

Closed
franciscop opened this issue Aug 17, 2023 · 4 comments · Fixed by #2602
Closed

Have client.connect() return a Promise<client> instead of Promise<void> #2601

franciscop opened this issue Aug 17, 2023 · 4 comments · Fixed by #2602

Comments

@franciscop
Copy link
Contributor

Motivation

It's not possible to create a client in a single step that defers the creation (even if async), so now if we need a promise of the client we need to do this:

const storeProm = (async() => {
  const client = createClient();
  await client.connect();
  return client;
})();
// Can use the client promise

If I want to use Redis as a KV store, it needs to be initialized and awaited:

const client = createClient();

// Need to await, which we might not yet be in the right place to do so
await client.connect();

const api = fch.create({ cache: { store: client } });

However it'd be nice if we could join both steps into one, so that it could be done in a single step without the extra syntax:

const store = await createClient().connect();
const api = fch.create({ cache: { store }});

More specifically, I have a library called Swear that is able to very nicely wrap the creation promise and avoid any await at all until the "leaf" function is called and awaited:

// Currently
const store = swear((async() => {
  const client = createClient();
  await client.connect();
  return client;
})());
await store.set('a', 'b');
await store.get('a');

// If .connect() returned a Promise<client>
const store = swear(createClient().connect());
await store.set('a', 'b');
await store.get('a');

Basic Code Example

// Would love to do this:
const store = await createClient().connect();
const api = fch.create({ cache: { store }});

// Which would also unlock also this with my library `swear`:
const store = swear(createClient().connect());
await store.set('a', 'b');
await store.get('a');
@franciscop
Copy link
Contributor Author

franciscop commented Aug 17, 2023

I'd be happy to try giving it a stab at it, I believe it would be here:

// Before
connect(): Promise<void> {
    // see comment in constructor
    this.#isolationPool ??= this.#initiateIsolationPool();
    return this.#socket.connect();
}

// After
async connect(): Promise<RedisClientType> {
    // see comment in constructor
    this.#isolationPool ??= this.#initiateIsolationPool();
    await this.#socket.connect();
    return this;
}

@leibale
Copy link
Collaborator

leibale commented Aug 17, 2023

I like this idea! :) Wanna open a PR?

BTW, don't forget to add a listener to error events as well, otherwise it will crash your application (see here)

@bigDP
Copy link

bigDP commented Nov 28, 2023

Hi, Can i take this task?

@leibale
Copy link
Collaborator

leibale commented Nov 28, 2023

@bigDP This is already done in #2602 (and should've been closed when I merged it, which did not happen for some reason). Sorry for the confusion, and thanks for wanting to contribute! :)

@leibale leibale closed this as completed Nov 28, 2023
@leibale leibale linked a pull request Nov 28, 2023 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants