Added parameter for app display name for dynamic rendering in the wallet during web5 connect flow

[thehenrytsai]

Currently there is no way for the wallet to dynamically display to the end-user a friendly display name when displaying the permission request. This PR introduces an app specified property for this purpose.

Out of scope for this PR, but a few items to consider for the future.

1. Consider adding a friendly name property in protocol definition for even better UX.
2. UX would be simpler/cleaner if app just asks delegated permission for a protocol, not enumerating through all the read, query, write etc.
3. If the permission request (coming from a website) is signed by a did:web DID or similar instead of an ephemeral DID, we'd be able to go the extra mile of perhaps matching the display name with the domain name and give user more assurances.

Tested against modified react starter and my own wallet.

PS. Kept some logging I added while debugging, happy to remove if preferred.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5087f2a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@web5/agent	Patch
@web5/api	Patch
@web5/user-agent	Patch
@web5/proxy-agent	Patch
@web5/identity-agent	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

TBDocs Report

✅ No errors or warnings

@web5/api

• Project entry file: packages/api/src/index.ts

@web5/crypto

• Project entry file: packages/crypto/src/index.ts

@web5/crypto-aws-kms

• Project entry file: packages/crypto-aws-kms/src/index.ts

@web5/dids

• Project entry file: packages/dids/src/index.ts

@web5/credentials

• Project entry file: packages/credentials/src/index.ts

---

TBDocs Report Updated at 2024-10-04T20:03:57Z 5087f2a

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.50%. Comparing base (b747a2d) to head (5087f2a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #945   +/-   ##
=======================================
  Coverage   93.49%   93.50%           
=======================================
  Files         117      117           
  Lines       33456    33468   +12     
  Branches     2706     2706           
=======================================
+ Hits        31281    31293   +12     
  Misses       2136     2136           
  Partials       39       39           

Components	Coverage Δ
agent	87.59% <100.00%> (+<0.01%)	⬆️
api	99.61% <100.00%> (+<0.01%)	⬆️
common	98.68% <ø> (ø)
credentials	94.95% <ø> (ø)
crypto	93.79% <ø> (ø)
dids	97.77% <ø> (ø)
identity-agent	96.42% <ø> (ø)
crypto-aws-kms	100.00% <ø> (ø)
proxy-agent	96.42% <ø> (ø)
user-agent	96.57% <ø> (ø)

[shamilovtim]

As web5-js is a package intended to be consumed by thousands of developers we can't expect them to appreciate us using their console for our debug messages. It it also unorthodox. If every node_module we had in the project was using our console for their debug messages we wouldn't appreciate that, we should have a similar level of respect for users of a package we're authoring.

[LiranCohen]

Great work, but agree with Tim I don't think we need to include a ton of logging here.

However I do think we should include some optional callbacks in various state of the flow in order to allow a wallet/app to display status changes as things are happening and/or log something themselves when testing the flow.

[thehenrytsai]

Good call on keeping console log clean by default. Was hoping to wait until we have a more thought-through logger to use, but added a logger that doesn't log by default, we can iterate over the logger as needed.

Happy to separate the logger portion into a separate PR, kept the logs in this PR to show the context why/when they were introduced.

Optional callbacks in various state of the flow would be cool, but probably scope for a different PR.

[shamilovtim]

Good call on keeping console log clean by default. Was hoping to wait until we have a more thought-through logger to use, but added a logger that doesn't log by default, we can iterate over the logger as needed.

Happy to separate the logger portion into a separate PR, kept the logs in this PR to show the context why/when they were introduced.

Optional callbacks in various state of the flow would be cool, but probably scope for a different PR.

This ships a Node.js logger into the web5-js bundle in production, which is inappropriate for clients and servers consuming a library. Beyond Node.js we also support browser, react-native, electron and any other platform that runs a Javascript engine. The best way of accomplishing what you want are callbacks like @LiranCohen suggested. Beyond that (and less important) it's worth it to point out that since this is a library rather than a backend service it's inappropriate to have debug lines of code bloating the LOC count.

[thehenrytsai]

@shamilovtim, Imma rip this out into a separate PR since the main goal is go rid of hardcoded app name in the wallet.

I will say that I tested the logger works in browser, but taking in your suggestion, I'll just hand-role a few lines for the implementation without depending on an external lib, unless you have one that you want to recommend. The logger interface is there for exactly the purpose of swapping out implementation easily.