Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message tab, rewrite using connections. #143

Closed
jcblw opened this issue Jun 12, 2021 · 1 comment · Fixed by #154
Closed

Message tab, rewrite using connections. #143

jcblw opened this issue Jun 12, 2021 · 1 comment · Fixed by #154
Labels
core issue or PR for the core of finch-graphql package. devtools issue or PR for the finch-graphiql-devtools package.

Comments

@jcblw
Copy link
Contributor

jcblw commented Jun 12, 2021

I was wanting to use to dogfood finch in this instance but I think the way current messages works does not work well for what we are trying to do with message tabs.

Current implementation issues

Message polling

The biggest issue is that we base the messages on polling, and the extension holding onto a small amount of cache. Once the dev tools extension polls for the messages of the extensions it will essentially pull that cache and remove it from the extension.

The first issue is that if there are two instances of Finch GraphiQL polling these messages it means that there is a good chance the extension will only get partial messages because one of the extension instances may be getting a majority of the cache and clearing it from the others.

The second issue is that this barrage of messages is probably not great for the perf of the extension. I have not seen too much slowdown that I can attribute to this issue, but I assume it will probably be happenings.

Recording immediately

This is probably just me being lazy with the initial implementation but in this instance, once the dev tools open the polling starts. This might be a combo on how chakra-ui tabs work, but sure there is a hook we can use to know if the tab is open. We should probably only record when the tab is open to avoid any unwanted polling.

Next version of message tab.

Moving to ports

Message tabs should move to a port implementation so that way we can push to the dev tools. This change alone gets rid of polling and the issue of storing cache. It also allows us to potentially send multiple messages per each request. We can eventually do timing and not show requests after response, and actually show them based on when they initially get sent.

Recording only when the tab is open.

This makes it super simple to know when you should be receiving messages. I think the initial open will start recording but we should also have a way to turn off the recording. Also, it would be nice once the recording is started it will persist between tabs. Until turned off or the extension is closed.

Downsides

This is probably going to affect Toucan, but if we do not handle connections super well we have the potential to create targets that going to be getting random information. This should be on a list of breaking changes for finch-graphql once published.

@jcblw jcblw added devtools issue or PR for the finch-graphiql-devtools package. core issue or PR for the core of finch-graphql package. labels Jun 12, 2021
@jcblw
Copy link
Contributor Author

jcblw commented Jun 14, 2021

Things already addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core issue or PR for the core of finch-graphql package. devtools issue or PR for the finch-graphiql-devtools package.
Development

Successfully merging a pull request may close this issue.

1 participant