Support co-editing Flutter and Android in a single project #3850

stevemessick · 2019-09-05T16:16:10Z

Replacement for #3817, due to git merge error.

Caveat: only works for Android Studio; IntelliJ support will be added later.

TODO: Find a better way to manage Maven repositories in IntelliJ. The project structure dialog is way too slow.
TODO: Clean up module dependencies after Android Q is released.

There is a comment in AndroidUtils about Add-to-app notes. It will be deleted before the co-edit work is finalized (after Android Q) but I want it to stay for now. Likewise, #triggerSyncIfFlutterModuleFoundAfterBuild has been commented-out and will probably be removed, but there's a (rare?) use case that might require it.

stevemessick · 2019-09-05T21:50:29Z

This is ready for review.

@devoncarew @pq @jacob314 for review or suggestions of reviewers.

@alexander-doroshko there's a solution for https://youtrack.jetbrains.com/issue/WEB-40082 embedded in this PR.

@xster this handles the modules that are added using include_flutter.groovy; following PRs will adapt it to work with post-source modules.

@DaveShuckerow and I will be looking at ways to move some of this out of IntelliJ and into a shareable service so VS Code can take advantage of it.

Android Q sources are out now but I will do the above-mentioned cleanup in another PR.

xster · 2019-09-06T02:31:33Z

src/io/flutter/utils/AndroidUtils.java

+  /*
+    Add-to-app notes:
+    - The Flutter module must be added to the Android app via manual editing as in the add-to-app docs,
+      or by using the forthcoming tool in Android Studio that generates a module and does the editing.


If I understand what this is potentially referring to, this is already possible via the new section in the wiki https://github.com/flutter/flutter/wiki/Add-Flutter-to-existing-apps#1-depend-on-the-android-archive-aar. Though tracing back to the source of the AAR is indeed going to be a challenge.

Thanks for taking a look, @xster.

xster · 2019-09-06T02:33:04Z

I can't review the plugin part but the concept LGTM.

devoncarew · 2019-09-09T16:52:16Z

Great to see this PR!

@DaveShuckerow, can you review? Apart from Steve, I think you're closest to the add2app story.

DaveShuckerow

Comments and questions. This first round of review, I'm trying to get a better picture of what's happening and why.

Thanks!

DaveShuckerow · 2019-09-05T22:16:31Z

src/io/flutter/FlutterInitializer.java

+      MessageBusConnection connection = project.getMessageBus().connect(project);
+      connection.subscribe(ProjectTopics.MODULES, new ModuleListener() {
+        @Override
+        public void moduleAdded(@NotNull Project proj, @NotNull Module mod) {


This module listener logic looks like it could be refactored out and shared with the occurrence in FlutterStudioStartupActivity.runActivity.

That's the plan, once the consolidation is possible. When I was working on this there were significant differences in IntelliJ and Android Studio. Now that Android Q is public I hope to be able to clean up some duplicated code, but that's not happening for a few weeks. The two code blocks you refer to have TODOs outlining the eventual changes. Note that FlutterInitializer runs in all IntelliJ-based IDEs but FlutterStudioStartupActivity only runs in Android Studio.

DaveShuckerow · 2019-09-10T17:18:09Z

src/io/flutter/utils/AndroidUtils.java

+  }
+
+  public static void addGradleListeners(@NotNull Project project) {
+    if (!FlutterUtils.isAndroidStudio()) {


Why is this not needed in Android Studio?

Android Studio has its own notion of how Gradle runs and broadcasts its events. The two are not compatible, at least today. It is possible that IntelliJ will adopt the Android Studio protocol in the future (and if they do they will likely find a way to not break existing clients).

DaveShuckerow · 2019-09-10T17:21:57Z

src/io/flutter/utils/AndroidUtils.java

+    });
+  }
+
+  private static boolean isVanillaAddToApp(@NotNull Project project, @Nullable VirtualFile file, @NotNull String name) {


What does 'vanilla' add to app mean?

No flavorings have been added, like files that have been edited or added. This method examines the structure of a project to ensure that 1) it looks like a Flutter android module and 2) has not previously been modified to run as an add-to-app module.

DaveShuckerow · 2019-09-10T17:32:55Z

src/io/flutter/utils/AndroidUtils.java

+    //  BuildModelContext.create(project).getOrCreateSettingsFile(project);
+    boolean isAndroidStudio = FlutterUtils.isAndroidStudio();
+    VirtualFile projectDir = requireNonNull(FlutterUtils.getProjectRoot(project));
+    Object param = isAndroidStudio ? projectDir.findChild(SdkConstants.FN_SETTINGS_GRADLE) : project;


This is a very interesting way of effectively creating a union type.

Is this equivalent to

if isAndroidStudio { VirtualFile param = projectDir.findChild(...); return BuildModelContext.getOrCreateSettingsFile(param); } else { return BuildModelContext.getOrCreateSettingsFile(BuildModelContext.create(project)); }

I'm trying to understand what's happening here.

It would be equivalent, if that code could be compiled. The API skew between IntelliJ and Android Studio means we have to use reflection here.

DaveShuckerow · 2019-09-10T17:33:58Z

src/io/flutter/utils/AndroidUtils.java

+
+  private static void addCoeditTransformedProject(@NotNull Project project) {
+    if (!project.isDisposed()) {
+      COEDIT_TRANSFORMED_PROJECTS.add(project);


Do we ever remove a project from this list?

I can see it is a WeakList, which I assume means that when the projects are disposed, the list will remove them. Let me know if that is not correct.

Correct, the weak reference allows a disposed project to get GC'd, and the garbage collector removes it from the collection. This will be an infrequent occurrence.

DaveShuckerow · 2019-09-10T17:44:07Z

src/io/flutter/utils/AndroidUtils.java

+
+  // Copied from org.jetbrains.plugins.gradle.execution.GradleRunAnythingProvider.
+  @NotNull
+  @SuppressWarnings("DuplicatedCode")


What is the difference between this and the gradle plugin code?

Accessibility. It happens a lot. Perfectly usable code is marked private and is unusable elsewhere.

DaveShuckerow · 2019-09-10T18:07:25Z

src/io/flutter/utils/AndroidUtils.java

+    return null;
+  }
+
+  private static class CoEditHelper {


My understanding is that the logic from here to the end of the file is focused on rewriting files and is something that we might want to share with the VS Code plugin, which is where we would write code in Dart as we discussed yesterday.

The places that dependencies couple this with the rest of the file are:

parseSettings()

findInclude() uses the Psi tree.

We'll need to understand the requirements for a VS Code application of the logic to identify how to prune, or if it's worth doing.

Correct. We'd also want to move the functionality of isVanillaAddToApp() into the external service. Once we've done that there won't be any dependencies left, since both parseSettings() and findInclude() are only used by isVanillaAddToApp() and enableCoediting().

stevemessick · 2019-09-10T20:18:11Z

Thanks @DaveShuckerow for taking a look at this. I know it isn't easy to understand. It was hard to write, too. It has to compile (and eventually run) in both IntelliJ and Android Studio, for both source and non-source dev environs, and across all supported products (in product-matrix.json). Up to now we've been lucky and haven't had too deep an integration with code that differs between AS and IJ but with the Gradle stuff we're jumping into the deep end, so to speak.

DaveShuckerow

This LGTM. Please include a comment near the reflection code explaining why it's necessary so that if we have to change it in the future it's a little clearer.

stevemessick · 2019-09-16T18:02:59Z

Sure, I'll add that to be included with the next PR. Thanks!

ThinkDigitalSoftware · 2019-10-01T17:51:45Z

Thanks, @stevemessick!

) * patch * Fix typo * Fix merge error * Debug tests

stevemessick added 2 commits September 5, 2019 06:31

patch

6e91261

Fix typo

ee28987

googlebot added the cla: yes label Sep 5, 2019

stevemessick mentioned this pull request Sep 5, 2019

[WIP] Support co-editing Flutter and Android in a single project #3817

Closed

stevemessick added 10 commits September 5, 2019 09:37

Fix merge error

5c287cb

Debug tests

d58e674

Debug tests

a0b6837

Debug tests

0088b94

Debug tests

58da815

Debug tests

ce5a8f0

Debug tests

be2c5eb

Debug tests

28f3af2

Debug tests

bbc44d1

Debug tests

25a2165

xster reviewed Sep 6, 2019

View reviewed changes

This was referenced Sep 6, 2019

[WIP] Update module template to make it editable in an Android app flutter/flutter#36486

Closed

project keeps forgetting its flutter, requiring dart to be enabled on startup #2993

Closed

devoncarew assigned DaveShuckerow Sep 9, 2019

DaveShuckerow reviewed Sep 10, 2019

View reviewed changes

stevemessick mentioned this pull request Sep 12, 2019

Build for 2019.3 EAP and 3.6 canary 11 #3872

Merged

DaveShuckerow approved these changes Sep 16, 2019

View reviewed changes

stevemessick merged commit 6cbe6a5 into master Sep 16, 2019

stevemessick deleted the co-edit-patch branch September 16, 2019 18:04

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

Support co-editing Flutter and Android in a single project (flutter#3850

1e754ad

) * patch * Fix typo * Fix merge error * Debug tests

Support co-editing Flutter and Android in a single project #3850

Support co-editing Flutter and Android in a single project #3850

Uh oh!

Conversation

stevemessick commented Sep 5, 2019

Uh oh!

stevemessick commented Sep 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xster commented Sep 6, 2019

Uh oh!

devoncarew commented Sep 9, 2019

Uh oh!

DaveShuckerow 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!

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!

stevemessick commented Sep 10, 2019

Uh oh!

DaveShuckerow left a comment

Choose a reason for hiding this comment

Uh oh!

stevemessick commented Sep 16, 2019

Uh oh!

ThinkDigitalSoftware commented Oct 1, 2019

Uh oh!

Uh oh!