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

Disable hot reload profile/release mode #4276

Closed
johnpryan opened this issue Nov 18, 2022 · 4 comments
Closed

Disable hot reload profile/release mode #4276

johnpryan opened this issue Nov 18, 2022 · 4 comments
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps is bug
Milestone

Comments

@johnpryan
Copy link

The hot reload and hot restart buttons should be disabled when you run in profile or release mode, since they won't work. See flutter/flutter#37987.

Screenshot 2022-11-18 at 8 55 31 AM

@DanTup
Copy link
Member

DanTup commented Nov 19, 2022

I think the Hot Reload button used to use the presence of the service extension to be enabled, but maybe when adding support for hot-reloading Dart (non-Flutter) the condition was removed. I guess it should be "(is Flutter and service extension is available) || is not Flutter".

As for Restart, we can't actually disable that button, it's not a "Hot Restart" button but rather just a "Restart Debugging session" button that in the case of Flutter works by doing a Hot Restart (since it's faster). However, it should work because it should just stop the existing session (closing the app) and then restart it (a new instance of flutter run). I think the problem is that at the time when we need to tell VS Code whether we're handling the restart or not (during initialisation), we haven't been told whether it's preview/release mode. I'll look for some way to improve this.

@DanTup DanTup added this to the v3.54.0 milestone Nov 19, 2022
@DanTup DanTup added is bug in flutter Relates to running Flutter apps in debugger Relates to the debug adapter or process of launching a debug session and removed is enhancement labels Nov 19, 2022
@johnpryan
Copy link
Author

Sounds good to me 👍

@DanTup
Copy link
Member

DanTup commented Nov 23, 2022

I've pushed a change that hides/disabled Hot Reload for Flutter apps when the reloadSources extension is not available (it remains enabled/visible for Dart projects since they don't require this service and work via a standard VM Service API).

However, restart is more complicated. We can't hide the "restart" button (which we usually use for Hot Restart) because VS Code doesn't allow it (restart should always be possible, even if it's not a Hot Restart, because it should just be the same as stopping the session and starting it again). We also can't currently tell VS Code to handle the restart because we only know about profile/release flags in launchRequest (not during startup when capabilities are exchanged).

So there are a few options:

  1. Do nothing. This means the restart button on the debug toolbar will not work, and instead prints an error to the console in profile/release mode:

    Failed to Hot Restart: DebugAdapterException: Exception: Restart is not supported in profile mode

  2. Handle restartRequest in the DAP and terminate the flutter run process, clean up all state, and then restart it (this is probably the best solution, but also complex)
  3. Pass an additional flag for profile/release mode to the flutter debug_adapter process so it's known before launchRequest, and use that to set the supportsRestartRequest capability.

1 is ofc easiest, but I think we should probably do 2.

(on a related note, we should ensure Hot Restart commands are disabled/hidden from the palette etc., since we're just implementing a standard restart here, and having Hot Restart commands show up may be confusing where it's not supporting and we will do a cold restart).

@DanTup
Copy link
Member

DanTup commented Nov 23, 2022

Since the restart change above requires DAP changes, but the Hot Reload change will ship in the next VS Code release, I'm splitting this into two issues. This one can be for the Hot Reload change, and #4281 tracks handling Restart for profile/release.

(so I'm closing this one as done by ecad283)

@DanTup DanTup closed this as completed Nov 23, 2022
@DanTup DanTup changed the title Disable hot reload and hot restart buttons in profile/release mode Disable hot reload profile/release mode Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps is bug
Projects
None yet
Development

No branches or pull requests

2 participants