Clean up some redundant logic for connecting to the client#1392
Clean up some redundant logic for connecting to the client#1392jerelmiller merged 10 commits intomainfrom
Conversation
#508 Bundle Size — 1.27MiB (-0.04%).Warning Bundle contains 12 duplicate packages – View duplicate packages Bundle metrics
|
| Current #508 |
Baseline #507 |
|
|---|---|---|
 Initial JS |
1.23MiB(-0.04%) |
1.23MiB |
 Initial CSS |
0B |
0B |
 Cache Invalidation |
3.42% |
92.31% |
| Chunks |
5 |
5 |
| Assets |
12 |
12 |
| Modules |
946 |
946 |
| Duplicate Modules |
45 |
45 |
| Duplicate Code |
3.87% |
3.87% |
| Packages |
160 |
160 |
| Duplicate Packages |
9 |
9 |
Bundle size by type 1 change 1 improvement
| Current #508 |
Baseline #507 |
|
|---|---|---|
| JS |
1.23MiB (-0.04%) |
1.23MiB |
| IMG |
35.85KiB |
35.85KiB |
| HTML |
810B |
810B |
| Other |
778B |
778B |
Bundle analysis report Branch jerel/better-connect-timeout Project dashboard
| * Attempt to find the client on a 1-second interval for 10 seconds max | ||
| */ | ||
| let interval: NodeJS.Timeout; | ||
| function findClient() { |
There was a problem hiding this comment.
We no longer need this at all since we send a registerClient message any time the client is registered which will connect it for us.
| if (hook.ApolloClient) { | ||
| sendHookDataToDevTools("connectToDevtools"); | ||
| } else { | ||
| findClient(); |
There was a problem hiding this comment.
The devtools script will now try to connect and then wait up to 10 seconds for a registerClient or connectToDevtools message before giving up and showing the not found modal. Since we can rely on those messages instead, we don't need to try and find a client when this connectToClient message is sent.
In the future, this connectToClient could likely be replaced with an RPC call to simplify this a bit further. This adds a bit of complexity though to do this in this PR so I'll do this separately to keep this PR clean.
| connectedToPanel = true; | ||
| } | ||
|
|
||
| if (devtoolsMachine.state.value === "initialized") { |
There was a problem hiding this comment.
This is no longer needed since the rest of the messages are now robust enough to not have to "retry" connecting when the panel is first shown 🎉
alessbell
left a comment
alessbell
left a comment
There was a problem hiding this comment.
Nice that we no longer need findClient! 🚀
jerelmiller
force-pushed
the
jerel/better-connect-timeout
branch
from
1aa093f to
8e2e206
Compare
2 participants
With the updated registration mechanism added in #1375, we can simplify the way the connect timeouts are handled and remove some redundant logic for trying to find the client. This PR simplifies this mechanism to remove some redundancy.