MaikuB / MaikuB/flutter_local_notifications

Refactor the internal APIs to reduce boilerplate and casts

Open
#2,467 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Dart
Stars
2.7k
Forks
1.6k
Avg merge
4d 8h
Merged PRs (30d)
4

Description

Here are the relevant parts of the current API I'm looking to change
- details are defined in an implementation-specific package and are completely unrelated between platforms
- there is a platform plugin abstract class defined in its own package
- each platform implements their plugin in a separate package
- each plugin expects its own details
- At launch, the platform plugin, if there is one, registers itself as `FlutterLocalNotificationsPlatform.instance`
- the main plugin defines a "collection of details"
- the main plugin defines a "master plugin" that uses the correct details in the correct plugin

The key point of this issue is that there is no way to abstract over any of this. Since all the details types are completely unrelated, each plugin must override each `FlutterLocalNotificationsPlatform` method to accept its own version of the details, and the master plugin must also override each one to supply the platform implementation with the correct details. In other words, we get a lot of functions like

```dart
Future show(...) {
if (defaultTargetPlatform == TargetPlatform.android && ...) {
// call resolve() with details.android
} else if (defaultTargetPlatform == TargetPlatform.ios && ...) {
// call resolve() with details.ios
} else if (...) {
// and so on
}
}
```

This class alone takes up 500 lines of code and is full of runtime checks. I believe there's a better way:
#### In the `platform_interface` package:
- declare a base class for all details, `DetailsBase`
- declare a "collection of details", `DetailsCollectionBase`, with `T? forPlatform()`
- declare a base class for all plugins, `PluginBase` that uses `DetailsCollectionBase` when needed

Implementation of the platform interface

```dart
// ignore_for_file: public_member_api_docs, one_member_abstracts

abstract class DetailsBase { }

abstract class DetailsCollectionBase {
T? forPlatform();
}

abstract class PluginBase {
static PluginBase? instance;

void show(int id, covariant DetailsCollectionBase details);
}
```

#### In each platform's implementation/package:
- Subclass `DetailsBase` with that platform's details
- Subclass `PluginBase` and implement all the methods

Example of an Android implementation

```dart
// ignore_for_file: public_member_api_docs, one_member_abstracts, always_specify_types, avoid_print, lines_longer_than_80_chars

import 'platform.dart';

class AndroidDetails extends DetailsBase {
int get androidId => 3;
}

class AndroidPlugin extends PluginBase {
@override
void show(int id, DetailsCollectionBase details) {
final androidDetails = details.forPlatform();
print(androidDetails?.androidId);
}
}
```

#### In the main package:
- import all the platform implementations
- implement `DetailsCollectionBase` with a member for each platform, like `NotificationDetails`
- Implement `Plugin` by forwarding all methods to `PluginBase`
- Alternatively, just expose `PluginBase.instance` for end-users directly

Implementation of the main package

```dart
// ignore_for_file: public_member_api_docs, one_member_abstracts, always_specify_types, avoid_print, lines_longer_than_80_chars

import 'dart:io';

import 'android.dart';
import 'platform.dart';

class DetailsCollection extends DetailsCollectionBase {
DetailsCollection(this.android);
final AndroidDetails? android;

@override
T? forPlatform() {
if (Platform.isAndroid) {
return android as T?;
} else {
return null;
}
}
}

class Plugin extends PluginBase {
@override
void show(int id, DetailsCollection details) =>
PluginBase.instance?.show(id, details);
}
```

This would all be non-breaking, but a lot more code can be saved by making a slightly breaking change: moving all platform-specific parameters into their respective `PlatformNotificationDetails` or lifting them to the platform interface. For example, `.cancel(int id, {String? tag})` uses a tag parameter, and then specifically has to check whether to pass it (Linux) or not (otherwise). If all platforms accepted a `tag` parameter, this could be avoided. Similarly, `zonedSchedule` has an extra required parameter, `androidScheduleMode`, whereas that should maybe be an optional parameter on `AndroidNotificationDetails` with a reasonable default, like `.exact`.

@MaikuB What do you think? I'm in the middle of adding web support and I noticed a pattern like this would help reduce new code to the main package, and possibly some other boilerplate code as well, like the initialization settings. Feel free to pop any of these code samples into an IDE, they should have no analysis issues. Of course, I'll take care of the PR.

Contributor guide

Open the contributing guide

Research direction

Start in the platform_interface package by reviewing FlutterLocalNotificationsPlatform and the proposed DetailsBase, DetailsCollectionBase, and PluginBase entry points. Then inspect the main plugin's details collection and master plugin alongside each platform implementation. Done means the platform APIs share the proposed abstractions while preserving the required platform-specific details and behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
dart, flutter
Domain
desktop-dev, mobile-dev, web-dev
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.