This implements a basic context provider which connects to the identity search service
Depends on D10928
Details
Details
flow and verified connection by writing some basic code on web utilizing context provider
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- wyilio/prefix-client
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Can you explain which of these console.logs are going to get removed in a later diff, versus which ones are meant to stay long-term?
Comment Actions
Let's strip the logs for things we expect to happen
We should also consider the dev environment. If we expect the calls to fail in the dev failure, then we should find a way to silence the logs. Guessing it should work fine with staging though, right?
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 69 ↗ | (On Diff #36632) | I don't think we should keep this |
| 109 ↗ | (On Diff #36632) | I don't think we should keep this |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 110 ↗ | (On Diff #37017) | Please make sure all lines are no longer than 80 chars (except where forced by Prettier) |
| 115 ↗ | (On Diff #37017) | You should add a space after the colon here |
| 123 ↗ | (On Diff #37017) | Shouldn't this be identitySearchMessageToServerTypes? |
| 150 ↗ | (On Diff #37017) | I assume you'll need to add a method later to enable a consumer of this context to send messages (ie. search queries) across the socket? |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 115 ↗ | (On Diff #37017) | Actually not sure about this... how does console.log for native / web print two params? Does it already put some whitespace between them? If no, please add a space in all of the relevant spots here. If yes, you can ignore this feedback |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 115 ↗ | (On Diff #37017) | console.log automatically adds whitespace when there are multiple params |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 150 ↗ | (On Diff #37017) | Yep! This happens in https://phab.comm.dev/D11000 which was already reviewed |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 123 ↗ | (On Diff #37017) | Yeah. Thanks for catching this |
Comment Actions
Implementing the useGetIdentitySearchAuthMessage follows discussion from https://phab.comm.dev/D11059
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 75 ↗ | (On Diff #37415) | Do we need any sort of validation here on the client side? Guessing there's validation on the identity service |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 75 ↗ | (On Diff #37415) | Yeah. It's mainly validation on the identity service which then sends a response back whether or not the authMessage was valid or not. |