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

keyring-core and the providers duplicate much of w3up-client #295

Open
olizilla opened this issue Jan 26, 2023 · 1 comment
Open

keyring-core and the providers duplicate much of w3up-client #295

olizilla opened this issue Jan 26, 2023 · 1 comment

Comments

@olizilla
Copy link
Contributor

olizilla commented Jan 26, 2023

What if we significantly simplify our UI layer as minimal wrappers around w3up-client? We may sacrifice some modularity, but would reduce the surface area of what we support (and repeat) here in w3ui.

For context, the original decisions for what code lives in the headless components were made at a time when the w3up-client didn't have any of the functionality we needed. It now manages spaces and and "current space" state internally, which we duplicate here in keyring-core.

As an example... for #293 to export ucan delegations for a space I tried to save time and just copy the implementation from w3cli which use the createDelegation method on w3up-client. But our ui base doesn't provide an instances of w3up-client. So i could go grab the lower level delegate method from the access-client agent, but we don't expose the agent from keyring core. I will expose a method in keyring-core to do delegations, and chase it through react-keyring, so i can use it in w3console, but it also exposes that we might have more layers to w3ui than we can afford to maintain right now, and a review of our layering would be prudent.

aside: The name keyring-core should also be reviewed... it's a wrapper for an agent and spaces rather than a general key management thing.

@travis
Copy link
Member

travis commented Jan 26, 2023

+1 reducing layers and eliminating duplicative functionality!

The one note I'll add is that it would be nice to be able to opt out of (or at least plug into) the state management stuff in these libraries - UI-level code often wants its own state management semantics and I stubbed my toe a couple times on the spaces/currentSpace state being stored and managed inside our library when I really wanted something at the React level to handle it.

Concrete example: when I was working on the space creator, originally I wanted to have the entire space creation process happen in the left nav, but as soon as I called createSpace the right-hand-side Space UI changed because createSpace changes currentSpace. In this particular case I think the workaround is better than my original plan, but this was surprising behavior at the time and I think there are many other UI decisions that might be more complicated if we aren't careful about how much state management we do under the hood of our library without giving developers better hooks into state changes, etc.

travis added a commit that referenced this issue Nov 13, 2023
per #295 rework the keyring to use w3up-client rather than the lower level access library

I'd like to refactor these APIs soon, but for now I'm keeping them unchanged to minimize downstream changes
travis added a commit that referenced this issue Nov 20, 2023
per #295 rework the keyring
to use w3up-client rather than the lower level access library

I'd like to refactor these APIs soon, but for now I'm keeping them
unchanged to minimize downstream changes

I am releasing this as a breaking change - I don't think anything will
break, but would prefer to let downstream users opt in to this
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

No branches or pull requests

2 participants