TBD54566975 / web5-js

Monorepo for the Web5 JS TypeScript implementation
https://tbd54566975.github.io/web5-js/
Apache License 2.0
135 stars 56 forks source link

Allow web5.connect to work with a custom UserAgent #949

Open sondreb opened 1 month ago

sondreb commented 1 month ago

Note: @shamilovtim's Additions Users want to be able to provide custom optionality to the UserAgent used by Web5.connect We should be able to do this without bloating the params and argument drilling all of the params through multiple methods Therefore we could allow users to pass a custom Web5 User Agent in order to solve this problem This should eliminate the agentVault param


For example, to add dataPath as a parameter for Web5.connect() method. The dataPath should be forwarded to this request within the connect method:

const userAgent = await Web5UserAgent.create({ agentVault });

If this create method is provided with dataPath, it could be possible to have separate databases locally on the same app.

bnonni commented 1 month ago

Are you asking to bubble up dataPath as a direct arg in the Web5.connect() method call? Forgive me if I'm misunderstanding, but I'm pretty sure you don't need to add dataPath to .connect to achieve this. You use await Web5UserAgent.create() to do the same thing. https://github.com/TBD54566975/web5-js/blob/bfa0417a2e9fc1300c5e604bea19b75ab1c73645/packages/user-agent/src/user-agent.ts#L151-L155

Examples

  1. Multiple agents in one DATA folder

    const agent0 = await Web5UserAgent.create({ dataPath: 'DATA/AGENT_0' });
    const agent1 = await Web5UserAgent.create({ dataPath: 'DATA/AGENT_1' });
  2. Multiple DATA folders each with its own agent

    const agent0 = await Web5UserAgent.create({ dataPath: 'DATA_0/AGENT' });
    const agent1 = await Web5UserAgent.create({ dataPath: 'DATA_1/AGENT' });
  3. Passing a custom HdIdentityVault

    
    const dataPath0 = 'DATA/AGENT_0';
    const store0 = new LevelStore<string, string>({ location: `${dataPath0}/VAULT_STORE` });
    const agentVault0 = new HdIdentityVault({  store: store0 });
    const agent0 = await Web5UserAgent.create({ dataPath: dataPath0 });

const dataPath1 = 'DATA/AGENT_1'; const store1 = new LevelStore<string, string>({ location: ${dataPath1}/VAULT_STORE }); const agentVault1 = new HdIdentityVault({ store: store1 }); const agent1 = await Web5UserAgent.create({ dataPath: dataPath1, agentVault: agentVault1 });



Example 3 is redundant / unneeded if you don't want to pass your own custom vault because the `dataPath` arg in `.create` gets passed down to a `new HdIdentityVault` with a `LevelStore`.
https://github.com/TBD54566975/web5-js/blob/bfa0417a2e9fc1300c5e604bea19b75ab1c73645/packages/user-agent/src/user-agent.ts#L157-L160

However, if you want to pass your own custom vault, Example 3 is how you would do it.
sondreb commented 1 month ago

Are you asking to bubble up dataPath as a direct arg in the Web5.connect() method call?

Yes, just make it easier for consumers of the library to supple it, without having to replicate some of the logic within the connect method. Thanks for the examples given👍

bnonni commented 1 month ago

Are you asking to bubble up dataPath as a direct arg in the Web5.connect() method call?

Yes, just make it easier for consumers of the library to supple it, without having to replicate some of the logic within the connect method. Thanks for the examples given👍

@sondreb I understand. Makes sense. Obv I can't speak for the web5-js dev team, but speaking for myself, if I were them, I'd want to avoid overloading .connect method. It already has a sprawling list of args and does a LOT.

https://github.com/TBD54566975/web5-js/blob/bd1cb00616029b0d18687b597e90d3b7c4dbeae1/packages/api/src/web5.ts#L253-L264

But maybe adding it to one of the current args objects would solve your problem and keep the interface intact.

For example, the didCreateOptions object

export type DidCreateOptions = {
  /** Override default dwnEndpoints provided during DID creation. */
  dwnEndpoints?: string[];
 agentDataPath?: string; // Pass a custom dataPath down to Web5UserAgent.create and new HdIdentityVault
}

This small change would keep the .connect method arg interface consistent across versions while adding this optional var to pass into didCreateOptions to achieve your desired outcome.

Example implementation

const connection0 = await Web5.connect({
    password: 'password0',
    didCreateOptions: { agentDataPath: 'USER0_DATA/AGENT' } // or whatever you want
});

const connection1 = await Web5.connect({
    password: 'password1',
    didCreateOptions: { agentDataPath: 'DATA/USER_AGENT_1' }
});

From personal experience, this will move faster if you take the initiative to write the code and submit a PR :) Happy to collab with you on it in Discord if you want. Just lmk.

sondreb commented 1 month ago

Thanks for the reply @bnonni, though the connect function is already implemented using a structured parameter and doesn't require additional connect overloads. JavaScript doesn't support function overloading; your point would be valid if it was using basic arguments.

It's not a good alternative to put it on didCreateOptions, as this is not related to the DID; but the DWN.

Should be fairly easy to make a PR for this, you could give it a go if you want to, and I'll review and help?

bnonni commented 1 month ago

@sondreb Sorry, I misused the term overload. I simply meant, that this method has a lot of args already, and that can make the interface messy and confusing leading to bad dev experience, unexpected user behavior and/or unexpected use cases resulting in frustration. My comment was more of an intuition-based suggestion from a code design standpoint, less of a recommendation. That intuition comes from the fact that the .connect method is designed to be the entry point for the Web5 platform API, and when I design and implement APIs (web or otherwise) my top priority / goal is to keep the interface clean, simple and easy to use.

Regardless, I'll let you take lead on it. Feel free to ping me if you'd like review and help.

shamilovtim commented 1 month ago

@sondreb If we let you pass your own userAgent into the method would that solve this for you? I think it would be the most elegant solution here. Then you can add whatever optionality / params to your UserAgent that you want. It would also be healthier for the codebase since it would eliminate the agentVault param.

sondreb commented 1 month ago

Maybe it would be enough, at least making it a little bit easier than it is now. Thanks!

shamilovtim commented 1 month ago

@sondreb open for PRs until someone else gets to it if it's a priority for you. otherwise one of myself or @LiranCohen could eventually solve.

bnonni commented 1 month ago

@sondreb open for PRs until someone else gets to it if it's a priority for you. otherwise one of myself or @LiranCohen could eventually solve.

I'll give this a shot today.