Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@bparrishMines
Copy link
Contributor

@bparrishMines bparrishMines commented Mar 3, 2022

This is an experiment PR that:

  1. Overrides the dependencies in the example apps of webview_flutter and its platform implementations:

webview_flutter/example/pubspec.yaml:

dependency_overrides:
  webview_flutter_android:
    path: ../../webview_flutter_android
  webview_flutter_platform_interface:
    path: ../../webview_flutter_platform_interface
  webview_flutter_wkwebview:
    path: ../../webview_flutter_wkwebview

webview_flutter_android/example/pubspec.yaml:

dependencies:
  webview_flutter:
    path: ../../webview_flutter

dependency_overrides:
  webview_flutter_android:
    path: ../../webview_flutter_android
  webview_flutter_platform_interface:
    path: ../../webview_flutter_platform_interface

webview_flutter_wkwebview/example/pubspec.yaml:

dependencies:
  webview_flutter:
    path: ../../webview_flutter

dependency_overrides:
  webview_flutter_platform_interface:
    path: ../../webview_flutter_platform_interface
  webview_flutter_wkwebview:
    path: ../../webview_flutter_wkwebview
  1. Removes redundant integration tests (non-platform-specific tests) from implementation packages because webview_flutter/example now uses local versions.

  2. Implementation packages' examples now depend on webview_flutter. This mean that they don't need to maintain their own copy of the webview_flutter api in webview_flutter_platform/example/lib/web_view.dart.

The goals of this PR:

This PR can also be expanded to move platform implementation unit tests from webview_flutter_platform/tests to webview_flutter_platform/example/tests.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-android platform-ios labels Mar 3, 2022
@bparrishMines bparrishMines changed the title dependency overrrides maybe [WIP] dependency overrrides maybe Mar 3, 2022
@bparrishMines bparrishMines changed the title [WIP] dependency overrrides maybe [WIP] [webview_flutter] dependency overrrides maybe Mar 4, 2022
@bparrishMines bparrishMines changed the title [WIP] [webview_flutter] dependency overrrides maybe [WIP] [webview_flutter] dependency overrrides Mar 7, 2022
@stuartmorgan-g
Copy link
Contributor

  1. Overrides the dependencies in the example apps of webview_flutter and its platform implementations:

This would make breaking changes impossible; did you have a solution in mind for that?

  1. Removes redundant integration tests (non-platform-specific tests) from implementation packages because webview_flutter/example now uses local versions.
  2. Implementation packages' examples now depend on webview_flutter. This mean that they don't need to maintain their own copy of the webview_flutter api in webview_flutter_platform/example/lib/web_view.dart.

How would you test a feature that is being added to platforms but hasn't been exposed in the app-facing package?

These are the main reasons I haven't tried to do something like this. (My current thinking around reducing copies of tests is to extract a lot of shared code from the implementation package examples into a local-only package, so we don't need N copies. We'd still need two similar copies, one for the app-facing layer and one for the implementation layer, but I don't actually think that's fixable. We definitely need some kind of solution that's better than copying everything, but I expect it to need a design doc to weigh trade-offs.)

@bparrishMines
Copy link
Contributor Author

How would you test a feature that is being added to platforms but hasn't been exposed in the app-facing package?

These are the main reasons I haven't tried to do something like this. (My current thinking around reducing copies of tests is to extract a lot of shared code from the implementation package examples into a local-only package, so we don't need N copies. We'd still need two similar copies, one for the app-facing layer and one for the implementation layer, but I don't actually think that's fixable. We definitely need some kind of solution that's better than copying everything, but I expect it to need a design doc to weigh trade-offs.)

I agree and I started to realize these problems after I created this. I was working on a personal plugin and I was trying to find a solution to these issues without requiring someone to run a script that's in our repository. When we create a doc on best federated plugin practices, we can address this issue.

Closing since a solution to this needs more discussion.

@bparrishMines bparrishMines deleted the android_integration_webview branch March 31, 2022 20:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

p: webview_flutter Edits files for a webview_flutter plugin platform-android platform-ios

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants