Skip to content

Handle page refreshes and opening in new tabs #222

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

Merged
merged 11 commits into from
Mar 20, 2019
Merged

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented Mar 19, 2019

Fixes #202

Note that I added an app instance id - this allows us to unambiguously identify the correct tab for a given instance of an app, separate from the app id which is consistent across all instances.

We cache the debug services per app ID and keep them running, but we only allow debugging one instance of a given app at a time.

The rough logic here is as follows:

  • When a new client connects
    • if we have cached debug services for the app
      • if the old tab connection is still connected to the correct tab (page refresh case)
        • reconnect and create the new isolate
      • else
        • don't do anything
    • else
      • don't do anything
  • When a client disconnects (from the SSE connection)
    • destroy the isolate (but leave the proxy services running).
  • When a client sends a DevTools request
    • if we have an existing debug service for that app
      • if it is for the same app instance we are already debugging
        • open devtools tab pointing at the running debug services
      • else
        • alert the user that they can't debug the same app in multiple windows
    • else
      • create a new debug service for the app

@jakemac53 jakemac53 requested a review from grouma March 19, 2019 21:28
@grouma
Copy link
Member

grouma commented Mar 19, 2019

Discussed offline. This does not handle the webdev daemon scenario as it currently tries to connect with the appId streamed from the devHandler. Initial thought is to unblock this PR by streaming the ConnectRequest and using the appId / appInstanceId inside app_domain.dart. We will follow up with another PR that reuses much of this logic inside app_domain.dart so that we properly handle full page refreshes.

/// [appId] is a unique String embedded in the application available through
/// `window.$dartAppId`.
/// [appInstanceId] is a unique String embedded in the instance of the
/// application available through `window.$dartAppInstanceId`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment that an instance in this case is one to one with a tab?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It changes if there is a refresh in the same tab though - it unique per instance of the app.

@jakemac53 jakemac53 merged commit 55dc847 into master Mar 20, 2019
@jakemac53 jakemac53 deleted the page-refresh branch March 20, 2019 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants