This closes the identity search websocket connection if there is a null or changed auth message and the socket is still active.
Depends on D10938
Details
Details
flow and ran on web
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 42–43 ↗ | (On Diff #36876) | These two lines need to be moved to the effect. The code as-written is broken, as the component could render multiple times before the effect is run, which could result in authMessageChanged being false in the second run and the effect being skipped, even if authMessageChanged is true in the first run |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 60–62 ↗ | (On Diff #36876) | This looks broken as well.
|
Comment Actions
Can you rebase on master to fix the build failures?
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 42–46 | This code is definitely not idiomatic, as you can probably tell by the use of eslint-disable. It looks like you're trying to ignore updates to authMessage when none of its fields actually changed. This points to a larger issue here. Whoever is rendering IdentitySearchProvider is causing trouble by regenerating authMessage when none of its fields have changed. That component is probably where the React.useMemo belongs, but I can't tell because it doesn't appear like you render IdentitySearchProvider anywhere in your stack. It would make it a lot easier to review your code if you weren't introducing a component without introducing any usages of that component. In general, you should always look to introduce a usage for a new piece of code at the same time that you introduce that code. | |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 66 ↗ | (On Diff #37018) | If authMessageChanged and authMessage is truthy, how does the socket get restarted? I'm guessing the close call below triggers onClose, which flips the value of connected and triggers this effect again? |
| lib/identity-search/identity-search-context.js | ||
|---|---|---|
| 66 ↗ | (On Diff #37018) | Yes, exactly |