Skip to content

Show a warning when a module uses a v1 only plugin#44499

Merged
xster merged 5 commits intoflutter:masterfrom
xster:v1-plugin-warning
Nov 11, 2019
Merged

Show a warning when a module uses a v1 only plugin#44499
xster merged 5 commits intoflutter:masterfrom
xster:v1-plugin-warning

Conversation

@xster
Copy link
Member

@xster xster commented Nov 9, 2019

Description

Since v1 plugins can make incorrect assumptions when used in add-to-app, show a warning when they're being imported.

Related Issues

Fixes #44245
Fixes #44423

Tests

I added the following tests:

added tests in plugin_test

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 9, 2019
@xster xster requested a review from blasten November 9, 2019 03:15
Copy link
Member Author

Choose a reason for hiding this comment

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

@zoeyfan to double check on the wording here.

Copy link

Choose a reason for hiding this comment

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

Trivial nit: maybe move this closer to where is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, isModule happens to just be a pre-populated map key check, so it could be done in the loop

Copy link

Choose a reason for hiding this comment

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

what about a project that was created with v1 and ran make-host-app-editable?

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed on chat. If users generally consumed v1 in add-to-app, it would break in the next stable. Those plugins will need to be registered manually since our tools can't predict whether a v1 or v2 style consumption will be made in the host Android project.

Document in #36491

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

LGTM /w nit

@xster xster force-pushed the v1-plugin-warning branch from 3d8e551 to 52b9726 Compare November 11, 2019 22:01
@xster xster merged commit 9e0df25 into flutter:master Nov 11, 2019
@xster xster deleted the v1-plugin-warning branch November 11, 2019 23:56
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

4 participants