Skip to content
This repository has been archived by the owner on Mar 1, 2022. It is now read-only.

Feature/androidx ionic #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cabaird
Copy link

@cabaird cabaird commented Nov 22, 2019

I added code to my last hook in #85 to support Ionic v4, as their webview has an import to an old support library that breaks when androidx is enabled. This may not be something you want to take responsibility for in your plugin, and may rather leave this for the consumers of your plugin to handle considering you're not specifically an Ionic plugin.

I might suggest alternatively setting androidx support up as optional, and controlled by a variable at plugin install to allow for more users to be able to use it. This may be a safer option until Cordova and Ionic are fully switched over to androidx.

@darryncampbell
Copy link
Owner

darryncampbell commented Nov 23, 2019

Thank you @cabaird , rather than add a choice I would rather just have people who want Android X support use version 2.0.0 of this plugin. My question is that the code you added to this pull request compared to #85 seems to have implications beyond Ionic, will this have any negative knock-on effects to people developing with Cordova or Ionic apps that don't use the Webview? Sorry if that is a silly question but it has been a while since I did any real development with Cordova or Ionic.

@cabaird
Copy link
Author

cabaird commented Nov 25, 2019

In theory, I think this replace, and many others could be added to generically handle some lingering support imports based on the class mappings here. I feel a little dirty adding code to this plugin that directly alters the source files from other plugins, though, and feel like it may lead you down a rabbit hole. Maybe as long as your README is clear, you can let the consumers of this plugin make sure they're in full support of androidx with other tools as they need, and use 1.3 if not. I for example, added this replace in a hook, knowing that I need it.

@darryncampbell
Copy link
Owner

Thank you for the quick response & contributions. I feel the same as you, it doesn't feel right to alter other plugins' source code from this plugin so I'll just leave this pull request open so others can benefit from the code if they need it for androidx compatibility

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants