docs: Fix outdated READMEs for @ucanto/client and @ucanto/transport#388
Open
Dhruv-Varshney-developer wants to merge 7 commits intostoracha:mainfrom
Open
docs: Fix outdated READMEs for @ucanto/client and @ucanto/transport#388Dhruv-Varshney-developer wants to merge 7 commits intostoracha:mainfrom
Dhruv-Varshney-developer wants to merge 7 commits intostoracha:mainfrom
Conversation
alanshaw
requested changes
Aug 22, 2025
|
|
||
| const invocation = await Client.invoke({ | ||
| // Mock fetch that simulates a UCAN service | ||
| const mockFetch = async (url, input) => { |
Member
There was a problem hiding this comment.
I'm not sure if it's a good idea for a mock to be in the README, perhaps in a runnable code example in the repo. IMHO the README should shown how to use the library against an already running server - i.e. just the code you'd need to use the client. I'd link to the server README for actually setting up and running a server component.
| const issuer = ed25519.parse(process.env.CLIENT_KEYPAIR) | ||
|
|
||
| // Mock fetch that simulates a UCAN service | ||
| const mockFetch = async (url, init) => { |
Member
There was a problem hiding this comment.
Again, I'd save this for a runnable example in the repo, or better yet a runnable example with an actual server. Mocking the server like this is not great even for tests as it bypasses all delegation chain validation.
Note: in tests, you don't even need a HTTP server running - a Ucanto server is a channel, so you can pass it to the client as the channel to use.
…or object Co-authored-by: ash <alan138@gmail.com>
alanshaw
reviewed
Oct 23, 2025
| const issuer = ed25519.Signer.parse(process.env.CLIENT_KEYPAIR); | ||
| // Parse the service DID (public key) | ||
| // SERVICE_DID should be a DID like: did:key:z6Mkk89bC3JrVqKie71YEcc5M1SMVxuCgNx6zLZ8SYJsxALi | ||
| const service = ed25519.Verifier.parse(process.env.SERVICE_DID) |
Member
There was a problem hiding this comment.
Suggested change
| const service = ed25519.Verifier.parse(process.env.SERVICE_DID) | |
| const service = DID.parse(process.env.SERVICE_DID) |
NiKrause
added a commit
to NiKrause/ucanto
that referenced
this pull request
Nov 18, 2025
- Remove mock fetch from READMEs, show real server usage - Use DID.parse() instead of ed25519.Verifier.parse() for service DIDs - Convert client tests to use server-as-channel pattern - Add @ucanto/server and @ucanto/validator to client devDependencies
Merged
This was referenced Feb 11, 2026
hannahhoward
pushed a commit
that referenced
this pull request
Feb 16, 2026
**Addresses @alanshaw recommendations from PR #388** This branch combines PRs #388 and #392 and addresses all review comments from @alanshaw. ### Changes **README updates:** - Removed mock fetch functions from `@ucanto/client` and `@ucanto/transport` READMEs - Updated examples to show real server usage (via environment variables) - Changed `ed25519.Verifier.parse()` to `DID.parse()` for service DIDs (as suggested) - Added notes about using server-as-channel for testing, with links to server README **Test improvements:** - Converted `packages/client/test/client.spec.js` from mock fetch to server-as-channel pattern - Tests now validate delegation chains (no bypass) - Added `@ucanto/server` and `@ucanto/validator` to client devDependencies ### Benefits - Examples work with real servers (no mocks) - Tests properly validate UCAN delegation chains - Follows recommended patterns from the server package - All tests passing with 100% coverage maintained ### Related - Addresses review comments from: #388 - Should complete work for: #387 (which was respected in #392) --------- Co-authored-by: Dhruv-Varshney-developer <dhruvvarshney77@gmail.com> Co-authored-by: ash <alan138@gmail.com>
Contributor
|
i think this PR can be closed since of merged: #407 (which includes that PR) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partially completes work for #387 - Addresses user report of broken imports and wrong function calls in documentation.
Changes:
@ucanto/transport:
The old README had imports that don't exist (
@ucanto/transport/cbor) and showed wrong function calls. Rewrote it with actual working code that matches what's in the package. Also updated the descriptions to accurately reflect what the package actually does instead of generic text. Now developers can copy-paste and run examples instead of getting import errors.@ucanto/client:
The old README used outdated API calls (
ed25519.Verifier.parse()) and had incomplete examples with undefined variables. Fixed all the function calls to match current API and added the missing connection setup code. Now the examples actually work when you run them.Both packages:
packages/client/test/fixtures.jsso examples work immediatelyBefore: Developers tried the README examples and got errors because the code was wrong.
After: Examples run successfully and show how to actually use these packages.