Some initial work for FlutterWeb apps #3315

jwren · 2019-03-25T21:50:17Z

No description provided.

jwren · 2019-03-25T21:50:38Z

@jacob314 Some additional comments and questions will be via email/ our chat.

jacob314 · 2019-03-26T00:04:19Z

src/io/flutter/run/SdkFields.java

-    final FlutterCommand command = flutterSdk.flutterRun(root, main.getFile(), device, runMode, flutterLaunchMode, project, args);
+
+    final FlutterCommand command;
+    if(FlutterUtils.declaresFlutterWeb(root.getPubspec())) {


nit:

final FlutterCommand command = FlutterUtils.declaresFlutterWeb(root.getPubspec() ? flutterSdk.flutterRunWeb(root, runMode, false) : flutterSdk.flutterRun(root, main.getFile(), device, runMode, flutterLaunchMode, project, args);

is a little simpler

jacob314 · 2019-03-26T00:05:17Z

src/io/flutter/run/SdkRunConfig.java

    final RunMode mode = RunMode.fromEnv(env);
    final Module module = ModuleUtilCore.findModuleForFile(mainFile.getFile(), env.getProject());
    final LaunchState.CreateAppCallback createAppCallback = (device) -> {
-      if (device == null) return null;


Does removing this check cause any problems for regular flutter apps?
An alternate that might be more robust is to create a mock ChromeBrowser Device to use for FlutterWeb.

No, it does not as it can't be null unless it is a FlutterWeb execution.

Adding a comment to this effect.

jacob314 · 2019-03-26T00:06:49Z

src/io/flutter/run/daemon/FlutterApp.java

  private final @Nullable Module myModule;
  private final @NotNull RunMode myMode;
-  private final @NotNull FlutterDevice myDevice;
+  // TODO(github.com/flutter/flutter-intellij/issues/3293) myDevice is not-null for all run configurations except flutter web configurations


The bug listed doesn't cover the issue. e.g. whether Device should be null for FlutterWeb.

Right, the bug URL however is the searchable text that can be used to identify all locations in source that might want to be visited if this expectation switches (with a mocked Chrome device for instance.)

jacob314 · 2019-03-26T00:07:53Z

src/io/flutter/sdk/FlutterCommand.java

+  // TODO(github.com/flutter/flutter-intellij/issues/3293) This is a temporary "<pub-cache>/bin/webdev" that can be provided to test some
+  //  webdev directly.
+  @Nullable
+  private final String localWebDevExe = "webdev";


can this be removed before checkin?

That is up to you and Devon. I'd like to land it to allow you to verify that things work as expected on someone elses' machine.

If your question is: should this be removed before users use it, then yes. This string only exists for line 267 below to change the generation of the command line from FlutterSdk (below) from flutter packages pub global run webdev serve [--debug] [--hot-reload] to webdev serve. After the appropriate Dart SDK lands in the Flutter SDK, this field should be removed.

jacob314 · 2019-03-26T00:09:20Z

src/io/flutter/sdk/FlutterSdk.java

+    args.add("webdev");
+    args.add("daemon");
+    if (mode == RunMode.DEBUG) {
+      //args.add("--debug");


do these need to be commented out? I imagine they might apply if/when we support debug mode.

Removing the dead code and adding a comment.

jacob314 · 2019-03-26T00:11:04Z

src/io/flutter/server/vmService/DartVmServiceDebugProcess.java

-                        + e.toString() + "\n" +
-                        formatStackTraces(e));
-        return;
+      // TODO(github.com/flutter/flutter-intellij/issues/3293) The following check disables the WebSocket connection for all FlutterWeb


please add more granularity to this issue. How does this test fail.
If it is needed, check based on whether the app is a FlutterWebApp not based on the device being null.

I'll add call in FlutterApp isWebDev instead of checking for null, and a reference to dart-lang/webdev#233

jacob314

Great to see this land. a few minor comments and questions.

jacob314 · 2019-03-27T18:15:04Z

src/io/flutter/run/SdkRunConfig.java

    final RunMode mode = RunMode.fromEnv(env);
    final Module module = ModuleUtilCore.findModuleForFile(mainFile.getFile(), env.getProject());
    final LaunchState.CreateAppCallback createAppCallback = (device) -> {
+      // Up until the FlutterWeb support, device was checked for null and returned.  The device can only be null if this is a FlutterWeb


nit: wrap comments at 80 chars.

jacob314 · 2019-03-27T18:15:23Z

src/io/flutter/run/daemon/FlutterApp.java

  }

+  public boolean isWebDev() {
+    return myDevice == null;


nit: add a TODO that this is a temp hack.

This reverts commit 2595b1b.

@jacob314

* Some initial work for FlutterWeb apps * comments from @jacob314 * 2 more nit comments from @jacob314

…r#3328) This reverts commit 2595b1b.

Some initial work for FlutterWeb apps

03df0e6

googlebot added the cla: yes label Mar 25, 2019

jwren mentioned this pull request Mar 25, 2019

P0: IntelliJ can create, launch, connect and inspect a FlutterWeb app (a webdev based workflow) #3293

Closed

7 tasks

jacob314 reviewed Mar 26, 2019

View reviewed changes

comments from @jacob314

fa529c3

jacob314 reviewed Mar 27, 2019

View reviewed changes

jacob314 approved these changes Mar 27, 2019

View reviewed changes

2 more nit comments from @jacob314

bcb344f

jwren merged commit 2595b1b into flutter:master Mar 27, 2019

jacob314 added a commit to jacob314/flutter-intellij that referenced this pull request Mar 28, 2019

Revert "Some initial work for FlutterWeb apps (flutter#3315)"

9b6a053

This reverts commit 2595b1b.

jacob314 added a commit that referenced this pull request Mar 28, 2019

Revert "Some initial work for FlutterWeb apps (#3315)" (#3328)

1c447c3

This reverts commit 2595b1b.

jwren mentioned this pull request Apr 1, 2019

Take 2 on "Some initial work for FlutterWeb apps" change #3342

Merged

alexander-doroshko pushed a commit to alexander-doroshko/flutter-intellij that referenced this pull request Jan 24, 2020

Some initial work for FlutterWeb apps (flutter#3315)

5769802

* Some initial work for FlutterWeb apps * comments from @jacob314 * 2 more nit comments from @jacob314

alexander-doroshko pushed a commit to alexander-doroshko/flutter-intellij that referenced this pull request Jan 24, 2020

Revert "Some initial work for FlutterWeb apps (flutter#3315)" (flutte…

58ca247

…r#3328) This reverts commit 2595b1b.

jwren deleted the FlutterWeb-run-config-02 branch October 18, 2023 20:17

Some initial work for FlutterWeb apps #3315

Some initial work for FlutterWeb apps #3315

Uh oh!

Conversation

jwren commented Mar 25, 2019

Uh oh!

jwren commented Mar 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacob314 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants