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

[video_player] Remove default method channel implementation #6341

Merged

Conversation

stuartmorgan
Copy link
Contributor

Eliminates MethodChannelVideoPlayer, and instead makes the default a
no-op subclass of the interface (i.e., one that throws
UnimplementedError for everything). This moves the plugin from option
3 to option 2 in
https://docs.flutter.dev/go/platform-channels-in-federated-plugins.

This implementation is being removed because its use of Pigeon made
ongoing maintenance impractical. Since default implementations are
intended for backward compatibility with third-party native-only
implementations, the platform channel internals of default
implementations are considered part of the API surface. Pigeon considers
those details implementation details, which means it was impossible to
ever update the version of Pigeon used here without breaking changes.
This means that fundamentally, Pigeon is not a viable choice for default
implementations.

The alternative to removing it would be replacing it with a new
non-Pigeon method channel implementation, but that would also be a
breaking change, defeating the purpose of backward compatibility.
This would only benefit new third-party implementations, but new
implementations should implement the platform interface rather than
using the legacy default implementation.

Fixes flutter/flutter#110017

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.

Eliminates `MethodChannelVideoPlayer`, and instead makes the default a
no-op subclass of the interface (i.e., one that throws
`UnimplementedError` for everything). This moves the plugin from option
3 to option 2 in
https://docs.flutter.dev/go/platform-channels-in-federated-plugins.

This implementation is being removed because its use of Pigeon made
ongoing maintenance impractical. Since default implementations are
intended for backward compatibility with third-party native-only
implementations, the platform channel internals of default
implementations are considered part of the API surface. Pigeon considers
those details implementation details, which means it was impossible to
ever update the version of Pigeon used here without breaking changes.
This means that fundamentally, Pigeon is not a viable choice for default
implementations.

The alternative to removing it would be replacing it with a new
non-Pigeon method channel implementation, but that would also be a
breaking change, defeating the purpose of backward compatibility.
This would only benefit new third-party implementations, but new
implementations should implement the platform interface rather than
using the legacy default implementation.

Fixes flutter/flutter#110017
@stuartmorgan stuartmorgan added the override: allow breaking change Override the check preventing breaking changes to platform interfaces label Aug 31, 2022
Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

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

lgtm

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 2, 2022
@auto-submit
Copy link

auto-submit bot commented Sep 3, 2022

auto label is removed for flutter/plugins, pr: 6341, Failed to merge pr#: 6341 with OperationException(linkException: null, graphqlErrors: [GraphQLError(message: Base branch was modified. Review and try the merge again., locations: [ErrorLocation(line: 2, column: 3)], path: [mergePullRequest], extensions: null)]).

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 3, 2022
@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 4, 2022
@tarrinneal tarrinneal merged commit 3c72094 into flutter:main Sep 4, 2022
@stuartmorgan stuartmorgan deleted the video-player-remove-default-impl branch September 4, 2022 16:09
adam-harwood pushed a commit to adam-harwood/flutter_plugins that referenced this pull request Nov 3, 2022
…6341)

Eliminates `MethodChannelVideoPlayer`, and instead makes the default a
no-op subclass of the interface (i.e., one that throws
`UnimplementedError` for everything). This moves the plugin from option
3 to option 2 in
https://docs.flutter.dev/go/platform-channels-in-federated-plugins.

This implementation is being removed because its use of Pigeon made
ongoing maintenance impractical. Since default implementations are
intended for backward compatibility with third-party native-only
implementations, the platform channel internals of default
implementations are considered part of the API surface. Pigeon considers
those details implementation details, which means it was impossible to
ever update the version of Pigeon used here without breaking changes.
This means that fundamentally, Pigeon is not a viable choice for default
implementations.

The alternative to removing it would be replacing it with a new
non-Pigeon method channel implementation, but that would also be a
breaking change, defeating the purpose of backward compatibility.
This would only benefit new third-party implementations, but new
implementations should implement the platform interface rather than
using the legacy default implementation.

Fixes flutter/flutter#110017
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
…6341)

Eliminates `MethodChannelVideoPlayer`, and instead makes the default a
no-op subclass of the interface (i.e., one that throws
`UnimplementedError` for everything). This moves the plugin from option
3 to option 2 in
https://docs.flutter.dev/go/platform-channels-in-federated-plugins.

This implementation is being removed because its use of Pigeon made
ongoing maintenance impractical. Since default implementations are
intended for backward compatibility with third-party native-only
implementations, the platform channel internals of default
implementations are considered part of the API surface. Pigeon considers
those details implementation details, which means it was impossible to
ever update the version of Pigeon used here without breaking changes.
This means that fundamentally, Pigeon is not a viable choice for default
implementations.

The alternative to removing it would be replacing it with a new
non-Pigeon method channel implementation, but that would also be a
breaking change, defeating the purpose of backward compatibility.
This would only benefit new third-party implementations, but new
implementations should implement the platform interface rather than
using the legacy default implementation.

Fixes flutter/flutter#110017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App override: allow breaking change Override the check preventing breaking changes to platform interfaces p: video_player
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Address null safety exclusions in flutter/plugins
2 participants