parse-community / Parse-SDK-Flutter

The Dart/Flutter SDK for Parse Platform
https://parseplatform.org
Apache License 2.0
575 stars 186 forks source link

awesome_notifications causing problems with iOS #936

Closed cool2apps closed 1 year ago

cool2apps commented 1 year ago

New Feature / Enhancement Checklist

Current Limitation

With parse_server_sdk_flutter package version 5.1.0, awesome_notifications becomes a dependency and awesome_notifications causes compile errors when building iOS apps.

Feature / Enhancement Description

Is it possible to use parse_server_sdk_flutter package without any notifications package? For example if you don't want to use any notification features with parse server, than you don't need to add any notification package to your project. If you want to use notification features with parse server, than you can manually add awesome_notifications package as a dependency to your project, manually.

If it has to be added a notification package, is it possible to use flutter_local_notifications instead of awesome_notifications, which is easier to integrate with iOS.

Example Use Case

Not applicable.

Alternatives / Workarounds

None. I can't manage to compile iOS apps with awesome_notifications.

parse-github-assistant[bot] commented 1 year ago

Thanks for opening this issue!

mbfakourii commented 1 year ago

If possible send the errors

Is this your problem?

mtrezza commented 1 year ago

I think it could make sense to make the notifications package optional. There are other notification providers that one could use, so they don't need that dependency, but it should be available OOTB for convenience. Would it be possible to make this an optional module (not sure if Flutter has such a concept), like for example the optional adapters in Parse Server?

@cool2apps, could you please provide more details about the compile errors?

cool2apps commented 1 year ago

If possible send the errors

Is this your problem?

Yes.

cool2apps commented 1 year ago

I think it could make sense to make the notifications package optional. There are other notification providers that one could use, so they don't need that dependency, but it should be available OOTB for convenience. Would it be possible to make this an optional module (not sure if Flutter has such a concept), like for example the optional adapters in Parse Server?

@cool2apps, could you please provide more details about the compile errors?

These are the error messages. But I will send detailed compile errors later today. When I change minimum OS to iOS 13, then I get other errors.

On the other hand, if I can manage to compile by increasing to iOS 13, I don't want to increase minimum iOS version from 11 to 13, when I don't need notifications.

[!] CocoaPods could not find compatible versions for pod "awesome_notifications":
  In Podfile:
    awesome_notifications (from `.symlinks/plugins/awesome_notifications/ios`)

Specs satisfying the `awesome_notifications (from `.symlinks/plugins/awesome_notifications/ios`)` dependency were found, but they required a higher minimum deployment target.
mbfakourii commented 1 year ago

I think it could make sense to make the notifications package optional. There are other notification providers that one could use, so they don't need that dependency, but it should be available OOTB for convenience. Would it be possible to make this an optional module (not sure if Flutter has such a concept), like for example the optional adapters in Parse Server?

@cool2apps, could you please provide more details about the compile errors?

It seems that this problem has been solved in awesome_notifications pre-release version 0.7.5-dev.2+1

In my opinion, there is no need to make this optional, the problem is only in a certain package

mtrezza commented 1 year ago

Still, developer may want their own push notifications systems, that's common in Android, iOS. So I wonder whether this can be made optional to provide a lighter core package?

In broader terms, the SDK may benefit from a modular structure like our other SDKs where developers can choose to not install certain modules they don't need.

cool2apps commented 1 year ago

If possible send the errors

Is this your problem?

I still have this problem.

Here is the actual error messages I get:

Failed to build iOS app
Swift Compiler Error (Xcode): Using bridging headers with module interfaces is unsupported

Uncategorized (Xcode): Command SwiftDriver emitted errors but did not return a nonzero exit code to indicate failure

Encountered error while archiving for device.

I have tried a lot of combinations like awesome_notifications 0.7.5-dev.2+1, latest development version awesome_notifications: ^0.7.5-dev.3, changed to minimum target to iOS 13, revert back to iOS 11 (which latest development version does not require iOS 13). I get similar error messages and I failed to compile at iOS.

So I switched back to parse_server_sdk_flutter: ^5.0.1 to be able to compile it at iOS.

mbfakourii commented 1 year ago

Temporarily use the following method to release the new version of awesome_notifications

dependency_overrides:
  awesome_notifications: ^0.7.5-dev.3
mtrezza commented 1 year ago

Couldn't we upgrade the dependency in the Parse Flutter SDK?

cool2apps commented 1 year ago

Temporarily use the following method to release the new version of awesome_notifications

dependency_overrides:
  awesome_notifications: ^0.7.5-dev.3

I got the same error with this dependency override.

cool2apps commented 1 year ago

By the way, I noticed that according to parse server SDK documentation, parse_server_sdk_flutter package is not compatible with flutter 3.10. And I was using flutter 3.10.5 when I get those error messages.

cool2apps commented 1 year ago

But to use Awesome Notifications FCM on release mode without the watermark [DEMO], you need to purchase a license key.

Awesome Notifications FCM plugin is not free and need to be paid to have an app without a demo watermark.

Please make parse_server_sdk_flutter package Awesome Notifications free. Notification package should be optional and should be choosable by the developer of the project. At my previous project, I implemented notifications with parse_server_sdk_flutter, flutter_local_notifications and firebase_messaging packages free and without any watermark and working fine.

mbfakourii commented 1 year ago

But to use Awesome Notifications FCM on release mode without the watermark [DEMO], you need to purchase a license key.

Awesome Notifications FCM plugin is not free and need to be paid to have an app without a demo watermark.

Please make parse_server_sdk_flutter package Awesome Notifications free. Notification package should be optional and should be choosable by the developer of the project. At my previous project, I implemented notifications with parse_server_sdk_flutter, flutter_local_notifications and firebase_messaging packages free and without any watermark and working fine.

If possible, test this

dependency_overrides:
  awesome_notifications: ^0.6.21
cool2apps commented 1 year ago

But to use Awesome Notifications FCM on release mode without the watermark [DEMO], you need to purchase a license key. Awesome Notifications FCM plugin is not free and need to be paid to have an app without a demo watermark. Please make parse_server_sdk_flutter package Awesome Notifications free. Notification package should be optional and should be choosable by the developer of the project. At my previous project, I implemented notifications with parse_server_sdk_flutter, flutter_local_notifications and firebase_messaging packages free and without any watermark and working fine.

If possible, test this

dependency_overrides:
  awesome_notifications: ^0.6.21

I will test and report back when I have time but I totally dropped Awesome Notifications in my mind because I learned that Awesome Notifications FCM is a paid package. We need to pay to get an app without demo watermark.

I wonder what is the logic behind to make a non free package a dependency to a free package such as parse server sdk.

mbfakourii commented 1 year ago

I checked the flutter_local_notifications library. this library requires items in the Android and iOS folders and files, which makes using this library in this package challenging.

mbfakourii commented 1 year ago

In this PR I try to go to flutter_local_notifications

mtrezza commented 1 year ago

At my previous project, I implemented notifications with parse_server_sdk_flutter, flutter_local_notifications and firebase_messaging packages free and without any watermark and working fine.

I think this speaks to the point I made earlier where an adapter concept for the SDK would be beneficial. We do have such concepts in other products (Parse Server, Parse Apple SDK, etc).

Awesome Notifications FCM plugin is not free and need to be paid to have an app without a demo watermark.

@cool2apps Could you please give a reference for this? AN is released under the Apache 2 license; the same license under which the Parse Flutter SDK is released. As I see it, for both to use them in your project you would be required to add an attribution note, that describes that your product in based on / using these products. The form in which you do that somewhat up to you (however, see license, there are some specific conditions).

mbfakourii commented 1 year ago

I think this speaks to the point I made earlier where an adapter concept for the SDK would be beneficial. We do have such concepts in other products (Parse Server, Parse Apple SDK, etc).

In general, there are two very famous and widely used libraries for notification in Flutter, the most famous and best of which is flutter_local_notifications which we use this package in ParseNotification.

ParseNotification is only in Flutter package and it is a basic part of Parse SDK Flutter which is also present in Parse-SDK-Android.

I don't think another package is needed for this because we want to use the best package.

In Flutter and Dart, when a program is compiled, only the code that is actually used during runtime is executed, and unused code is removed. In other words, regardless of how many classes or functions are defined in a package, only the code that is actually used in the program is loaded into memory and executed.

Additionally, Flutter and Dart use tree shaking to remove unused code. Therefore, using packages that are not used in the program does not affect the program's performance.

So I think there is no need to worry about lighter core.

mtrezza commented 1 year ago

Alright, then let's just make sure the license is appropriate. Adapters however also have other benefits. In this case here with push notifications, a developer could add the specific notification plugin they want to use in their app. So it gives them options. And specifying an adapter interface makes it easier for others to develop and contribute new adapters. See the Parse Server API Mail Adapter for example, which is even an adapter with has adapters [sic].

cool2apps commented 1 year ago

At my previous project, I implemented notifications with parse_server_sdk_flutter, flutter_local_notifications and firebase_messaging packages free and without any watermark and working fine.

I think this speaks to the point I made earlier where an adapter concept for the SDK would be beneficial. We do have such concepts in other products (Parse Server, Parse Apple SDK, etc).

Awesome Notifications FCM plugin is not free and need to be paid to have an app without a demo watermark.

@cool2apps Could you please give a reference for this? AN is released under the Apache 2 license; the same license under which the Parse Flutter SDK is released. As I see it, for both to use them in your project you would be required to add an attribution note, that describes that your product in based on / using these products. The form in which you do that somewhat up to you (however, see license, there are some specific conditions).

awesome_notifications is not compatible with firebase_messaging and you need to use awesome_notifications_fcm instead to send cloud based push notifications. And awesome_notifications_fcm is not completely free. In free version there is a watermark shown. You need to pay to get rid of this watermark.

https://pub.dev/packages/awesome_notifications_fcm#-license-key

Local notifications using Awesome Notifications are always 100% free to use. And you can also test all push notifications features on Awesome Notifications FCM for free, Forever.

But to use Awesome Notifications FCM on release mode without the watermark [DEMO], you need to purchase a license key. This license key is a RSA digital signature, validated with private and public keys in conjunction with plugin versionings and your App ID / Bundle ID. Because of it, once the license key is generated for your app, its forever. It will never expires and do not require internet connection to be validated.

The price of a license key is $ 9.99 / App, and it contains:

Push Notifications without watermark
1 license Key, expandable to 4 id variations
Perpetual Licenses
2 Dedicated Support Meetings
1 Year exclusive support on Discord
1 Year of Free Updates
cool2apps commented 1 year ago

In this PR I try to go to flutter_local_notifications

Thanks. I will try.

mbfakourii commented 1 year ago

Alright, then let's just make sure the license is appropriate. Adapters however also have other benefits. In this case here with push notifications, a developer could add the specific notification plugin they want to use in their app. So it gives them options. And specifying an adapter interface makes it easier for others to develop and contribute new adapters. See the Parse Server API Mail Adapter for example, which is even an adapter with has adapters [sic].

Adapters seem logical on the nodejs side. But on the flutter side, we have to provide a new package for adapters, whose only job is to display a notification, and considering that flutter_local_notifications meets our needs, I don't think we need an adapter.

cool2apps commented 1 year ago

Alright, then let's just make sure the license is appropriate. Adapters however also have other benefits. In this case here with push notifications, a developer could add the specific notification plugin they want to use in their app. So it gives them options. And specifying an adapter interface makes it easier for others to develop and contribute new adapters. See the Parse Server API Mail Adapter for example, which is even an adapter with has adapters [sic].

Adapters seem logical on the nodejs side. But on the flutter side, we have to provide a new package for adapters, whose only job is to display a notification, and considering that flutter_local_notifications meets our needs, I don't think we need an adapter.

If it is possible to 2 new packages would be nice like these:

parse_server_awesome_notifications
parse_server_local_notifications

If anyone would like to use awesome_notifications, then they add parse_server_sdk_flutter and first package as dependencies. If anyone would like to use flutter_local_notifications, then they add parse_server_sdk_flutter and second package as dependencies. If anyone will not use any notifications, then they just add parse_server_sdk_flutter package as a dependency. And they don't need to deal with the requirements and troubles that come with notification packages.

On the other hand, I don't understand why notifications should be built in parse server package. I currently use 5.0.1 version of the package without any notifications inside. I configure and add some code inside my app for flutter_local_notifications package, and it is working fine with android. Not started to integrate with iOS yet. iOS part is a bit more complicated than android. So I decided to integrate to android first.

mbfakourii commented 1 year ago

Alright, then let's just make sure the license is appropriate. Adapters however also have other benefits. In this case here with push notifications, a developer could add the specific notification plugin they want to use in their app. So it gives them options. And specifying an adapter interface makes it easier for others to develop and contribute new adapters. See the Parse Server API Mail Adapter for example, which is even an adapter with has adapters [sic].

Adapters seem logical on the nodejs side. But on the flutter side, we have to provide a new package for adapters, whose only job is to display a notification, and considering that flutter_local_notifications meets our needs, I don't think we need an adapter.

If it is possible to 2 new packages would be nice like these:

parse_server_awesome_notifications
parse_server_local_notifications

If anyone would like to use awesome_notifications, then they add parse_server_sdk_flutter and first package as dependencies. If anyone would like to use flutter_local_notifications, then they add parse_server_sdk_flutter and second package as dependencies. If anyone will not use any notifications, then they just add parse_server_sdk_flutter package as a dependency. And they don't need to deal with the requirements and troubles that come with notification packages.

On the other hand, I don't understand why notifications should be built in parse server package. I currently use 5.0.1 version of the package without any notifications inside. I configure and add some code inside my app for flutter_local_notifications package, and it is working fine with android. Not started to integrate with iOS yet. iOS part is a bit more complicated than android. So I decided to integrate to android first.

We have ParseNotification class in other Parse SDKs. This class is used to display notifications in Parse. In some places, for example, push notification, we use this class, and in the case of some packages, I don't think it is necessary, most users use flutter_local_notifications, even Flutter itself has suggested this package in some places.

Also, this package is part of flutter favorites packages

mbfakourii commented 1 year ago

@cool2apps It seems this was a bad suggestion, I think we should roll back.

@mtrezza According to pubdev, we no longer support Web and Windows in Parse Flutter after this merge!

mbfakourii commented 1 year ago

I think the best thing is to leave the notification display to the developer

mtrezza commented 1 year ago

According to pubdev, we no longer support Web and Windows in Parse Flutter after this merge!

Is this just what the pub.dev analyzer erroneously thinks, or is that actually the case? Because we do have Windows OS in the CI when testing, right?

mbfakourii commented 1 year ago

According to pubdev, we no longer support Web and Windows in Parse Flutter after this merge!

Is this just what the pub.dev analyzer erroneously thinks, or is that actually the case? Because we do have Windows OS in the CI when testing, right?

No, it is not really supported because flutter_local_notifications does not support these two platforms!

mtrezza commented 1 year ago

But awesome_notifications is also not an alternative, because it's a commercial product (open-source just for marketing purposes it seems). Should we leave flutter_local_notifications added for now with a note that it's only supporting mobile platforms? And somehow change the code so that it won't affect the pub.dev analysis.

Could we add the notifications as an optional feature that only works for some platforms, while there is a base functionality that works for all platforms?

mbfakourii commented 1 year ago

But awesome_notifications is also not an alternative, because it's a commercial product (open-source just for marketing purposes it seems). Should we leave flutter_local_notifications added for now with a note that it's only supporting mobile platforms? And somehow change the code so that it won't affect the pub.dev analysis.

Could we add the notifications as an optional feature that only works for some platforms, while there is a base functionality that works for all platforms?

Yes, it is possible, I think it should be optional, I will try to implement this part

mtrezza commented 1 year ago

It would be amazing if this worked.