-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat!: use w3up-client in keyring #581
Conversation
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far!
test('registerSpace fails if no current space is set', async () => { | ||
const agent = await createAgent() | ||
await expect(agent.registerSpace('[email protected]')).rejects.toThrowError() | ||
expect(space.did().startsWith('did:key:')).toBe(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename this file?
@@ -85,6 +85,11 @@ export const AgentLoader = ({ | |||
return children | |||
} | |||
|
|||
/** | |||
* @deprecated use ClientLoader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change anyways, just remove it!
const controller = new AbortController() | ||
setRegisterAbortController(controller) | ||
|
||
try { | ||
await accessAuthorize(agent, email, { signal: controller.signal }) | ||
await c.authorize(email, { signal: controller.signal }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this not change to login
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HM you're right - it looks like authorize
wasn't removed, but I do think this should use login - will update!
}) | ||
setSpace(getCurrentSpaceInAgent(agent)) | ||
setSpaces(getSpaces(agent)) | ||
const registerSpace = async (email: string): Promise<void> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be renamed to provisionSpace
?
await account.provision(space.did() as DIDKey) | ||
setSpace(c.currentSpace()) | ||
setSpaces(c.spaces()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to throw if missing account or space.
setSpace(getCurrentSpaceInAgent(agent)) | ||
const c = await getClient() | ||
await c.setCurrentSpace(did as DID<'key'>) | ||
setSpace(c.currentSpace()) | ||
} | ||
|
||
const loadAgent = async (): Promise<void> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename loadClient
?
if (agent != null) return | ||
await getAgent() | ||
if (client != null) return | ||
await getClient() | ||
} | ||
|
||
const unloadAgent = async (): Promise<void> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename unloadClient
} | ||
|
||
const unloadAgent = async (): Promise<void> => { | ||
setSpace(undefined) | ||
setSpaces([]) | ||
setIssuer(undefined) | ||
setAgent(undefined) | ||
setClient(undefined) | ||
setAccount(undefined) | ||
} | ||
|
||
const resetAgent = async (): Promise<void> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename resetClient
?
@alanshaw re: a lot of this - I'd like to avoid changing the API here so we don't have to go update all the examples. I hate the idea of trying to design the next version of this API under pressure of launching, so I propose we make these changes without any changes to the interfaces these modules export and then fast follow with a breaking change that mostly just renames various functions (but also potentially reworks/deletes/adds functions that makes sense) what do you think? |
ACK, ok yes :) |
🤖 I have created a release *beep* *boop* --- ## [6.0.0](keyring-core-v5.1.0...keyring-core-v6.0.0) (2023-11-20) ### ⚠ BREAKING CHANGES * use w3up-client in keyring ([#581](#581)) ### Features * use w3up-client in keyring ([#581](#581)) ([bd5f341](bd5f341)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [7.0.0](uploader-core-v6.1.0...uploader-core-v7.0.0) (2023-11-21) ### ⚠ BREAKING CHANGES * use w3up-client in keyring ([#581](#581)) ### Features * use w3up-client in keyring ([#581](#581)) ([bd5f341](bd5f341)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [5.0.0](uploads-list-core-v4.1.0...uploads-list-core-v5.0.0) (2023-11-21) ### ⚠ BREAKING CHANGES * use w3up-client in keyring ([#581](#581)) ### Features * use w3up-client in keyring ([#581](#581)) ([bd5f341](bd5f341)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [7.0.0](react-keyring-v6.2.1...react-keyring-v7.0.0) (2023-11-21) ### ⚠ BREAKING CHANGES * use w3up-client in keyring ([#581](#581)) ### Features * use w3up-client in keyring ([#581](#581)) ([bd5f341](bd5f341)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Alan Shaw <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [6.0.0](solid-keyring-v5.0.1...solid-keyring-v6.0.0) (2023-11-21) ### ⚠ BREAKING CHANGES * use w3up-client in keyring ([#581](#581)) ### Features * add support for getting an account's plan ([#564](#564)) ([11023a4](11023a4)) * re-export Service from `react-keyring` ([#577](#577)) ([308816d](308816d)) * use w3up-client in keyring ([#581](#581)) ([bd5f341](bd5f341)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [6.0.0](vue-keyring-v5.0.1...vue-keyring-v6.0.0) (2023-11-21) ### ⚠ BREAKING CHANGES * use w3up-client in keyring ([#581](#581)) ### Features * add support for getting an account's plan ([#564](#564)) ([11023a4](11023a4)) * re-export Service from `react-keyring` ([#577](#577)) ([308816d](308816d)) * use w3up-client in keyring ([#581](#581)) ([bd5f341](bd5f341)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
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