Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaking(android): bump fcm@18.+ #19

Merged
merged 3 commits into from
Aug 29, 2020
Merged

Conversation

erisu
Copy link

@erisu erisu commented Aug 28, 2020

This bumps the Android's Firebase Cloud Messaging version to 18.+.
This is the latest and last version that supports the Android Support Libraries.

@erisu erisu changed the title feat(android): bump fcm@18.+ breaking(android): bump fcm@18.+ Aug 28, 2020
@jcesarmobile
Copy link

Since the version change requires code changes, I think the version should not be configurable anymore, otherwise if users use an older version it won't work.

@erisu
Copy link
Author

erisu commented Aug 28, 2020

@AdriVanHoudt, do you use the FCM_VERSION flag?

@AdriVanHoudt
Copy link

Jup we have it set to <variable name="FCM_VERSION" value="17.+" /> atm.

@erisu
Copy link
Author

erisu commented Aug 28, 2020

Ok, that was the default value of the previous plugin release. I am debating on removing this flag. As jcesarmobile mentioned.

Basically, any version higher than 18.+ would require the use of AndroidX and might require code changes.
Allowing people to change this value might be undesirable at this point because I think it would lead to issues.

I wonder if anyone has changed the value higher successfully with out changing the plugin source code in the process to make it work.

@AdriVanHoudt
Copy link

AdriVanHoudt commented Aug 28, 2020

I think we just have it set at that because it was the default :D
I'm not familiar with android, but I think android x would require all plugins to use android x?

I also don't think we use any other fcm stuff so breakage would be weird in our case.

@erisu
Copy link
Author

erisu commented Aug 28, 2020

Yes, if the project enabled AndroidX, any Android plugins that used the Android Support Library would break. The plugin developers would need to be update their plugins to use AndroidX instead.

There is an adapter plugin that would do a class mapping conversions. With this, unmaintained plugins might continue to work, depending on if there is no code changes required.

But, I do not plan this first release to force people to use AndroidX. I might make a separate branch/tag that would support the AndroidX path but that is not planned yet.

Anyways, AndroidX is out-of-scope for now and there is a lot of discussion on this topic in Cordova as what to be done...

If I remove this flag, it shouldn't affect you in your case.

@AdriVanHoudt
Copy link

Thanks for the info.
Your plan sounds great!

@jcesarmobile
Copy link

Current default is 17.0.+, FirebaseMessagingService's onNewToken was added on 17.1.0.
https://firebase.google.com/support/release-notes/android

So either document that it can only be greater or equal than 17.1.0 and lower or equal than 18.0.0, or fix the version to always use 18.0.0

@erisu erisu force-pushed the feat/android-firebase-messaging-18 branch from 1f96bff to 40b033f Compare August 28, 2020 16:58
docs/INSTALLATION.md Outdated Show resolved Hide resolved
@erisu erisu merged commit 1ae3428 into master Aug 29, 2020
@erisu erisu deleted the feat/android-firebase-messaging-18 branch August 29, 2020 03:39
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants