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

Do not create plugins from dev_dependencies #56591

Open
jonahwilliams opened this issue May 7, 2020 · 16 comments
Open

Do not create plugins from dev_dependencies #56591

jonahwilliams opened this issue May 7, 2020 · 16 comments
Labels
a: build Building flutter applications with the tool a: plugins Support for writing, building, and running plugin packages a: tests "flutter test", flutter_test, or one of our tests f: integration_test The flutter/packages/integration_test plugin P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team

Comments

@jonahwilliams
Copy link
Member

Currently the flutter tool creates a plugin for all package dependencies, even though in the dev_dependencies section. We'd like to remove this behavior, since it can lead to plugins used for testing accidentally getting included in released builds. This will be a breaking change, so we can proceed in several steps.

  1. Compute both the current and dependencies-only plugin list and use the current strategy. If discrepancies are found, print a warning that this will be disabled in a future release and how to fix.

  2. Compute both the current and dependencies-only plugin list and use the dependencies only strategy. If discrepancies are found, print a warning/notification that certain plugins have been disabled and include how to fix.

  3. Remove the current strategy, remove the warning.

@jonahwilliams jonahwilliams added the tool Affects the "flutter" command-line tool. See also t: labels. label May 7, 2020
@dnfield dnfield added the f: integration_test The flutter/packages/integration_test plugin label Jun 5, 2020
@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

I think, ideally, it would be possible to include or not include a plugin based on a build flag or somesuch.

for integration_test (formerly known as e2e), we need some way to include the plugin in builds that need it, and strip it out for builds that do not.

@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

IOW, it seems like it'd be very nice if the tool could do something like this: if a plugin is in dev_dependencies, ignore it unless building for a test target (do not add it to any generated files for "regular" builds).

We'd need some meaningful way to discriminate between test builds and non.

@dnfield dnfield added a: build Building flutter applications with the tool a: tests "flutter test", flutter_test, or one of our tests labels Dec 8, 2020
@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

/cc @blasten @jmagman who probably know a lot more about how or if this could work on Android and iOS.

@jmagman
Copy link
Member

jmagman commented Dec 8, 2020

Is this different from #49101 or #45714?

On the iOS side this would be easy to do if .flutter-plugins-dependencies indicated which were dev-only.

@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

It's very similar, although I guess this one is proposing a specific solution to it that's a bit different from what I'm suggesting.

@blasten
Copy link

blasten commented Dec 8, 2020

yeah, that can be added to .flutter-plugins-dependencies.

IOW, it seems like it'd be very nice if the tool could do something like this: if a plugin is in dev_dependencies, ignore it unless building for a test target (do not add it to any generated files for "regular" builds).

That probably requires generating a new plugin registrant based on which target the user is running. In Objective-C you could use compile-time constants passed to Xcode, right @jmagman ?

@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

When do we re-generate the GeneratedPluginRegistrant code?

@blasten
Copy link

blasten commented Dec 8, 2020

pub get, currently

@jmagman
Copy link
Member

jmagman commented Dec 8, 2020

if (androidPlatform) {
await _writeAndroidPluginRegistrant(project, plugins);
}
if (iosPlatform) {
await _writeIOSPluginRegistrant(project, plugins);
}
if (linuxPlatform) {
await _writeLinuxPluginFiles(project, plugins);
}
if (macOSPlatform) {
await _writeMacOSPluginRegistrant(project, plugins);
}

That probably requires generating a new plugin registrant based on which target the user is running

Why would the native-side app ever need to register dev_dependencies?

@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

Why would the native-side app ever need to register dev_dependencies?

Because for integration_test, it has to report test results up to the native side test harness - e.g. to XCTest or JUnit.

@jmagman
Copy link
Member

jmagman commented Dec 8, 2020

Then shouldn't the test target be handling the registration?

@dnfield
Copy link
Contributor

dnfield commented Dec 8, 2020

I guess that makes sense, then we would just need to exclude it from the generated plugin registrant

@jmagman
Copy link
Member

jmagman commented Dec 16, 2020

With the addition of integration_test in every pubspec this opts every app into a dependency on CocoaPods. pod install will always run immediately after a flutter create with no additional plugins added to the app and without any integration_test usage.

@dnfield dnfield added the P1 High-priority issues at the top of the work list label Dec 16, 2020
@dnfield
Copy link
Contributor

dnfield commented Dec 16, 2020

Raising priority on this.

@jmagman
Copy link
Member

jmagman commented Jan 20, 2021

I filed #74274 separately in case there's an integration_test-specific work-around, like hard-coding its exclusion, that doesn't involve all the plumbing of dev_dependencies.

@blasten
Copy link

blasten commented Jun 11, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: build Building flutter applications with the tool a: plugins Support for writing, building, and running plugin packages a: tests "flutter test", flutter_test, or one of our tests f: integration_test The flutter/packages/integration_test plugin P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team
Projects
Status: Future bugs/improvements
Development

No branches or pull requests

8 participants