hieuvp / react-native-fingerprint-scanner

Provide Fingerprint, Touch ID, and Face ID Scanner for React Native (Compatible with both Android and iOS)
https://www.npmjs.com/package/react-native-fingerprint-scanner
871 stars 297 forks source link

Update build.gradle #101

Closed SaeedZhiany closed 4 years ago

SaeedZhiany commented 4 years ago

Load Android Gradle Plugin conditionally and change 'compile' to 'implementation'

This wraps the Android Gradle plugin dependency in the buildscripts section of android/build.gradle in a conditional:

if (project == rootProject) {
    // ... (dependency here)
}

The Android Gradle plugin is only required when opening the project stand-alone, not when it is included as a dependency. By doing this, the project opens correctly in Android Studio, and it can also be consumed as a native module dependency from an application project without affecting the app project (avoiding unnecessary downloads/conflicts/etc).

for more info, you can refer to this thread and especially this comment.

mikehardy commented 4 years ago

for what it's worth as a fellow maintainer - this change to the style of using gradle has worked well for me in react-native-device-info for a month, zero related issues, and should help harmonize gradle versions over time as the ecosystem adopts it https://github.com/react-native-community/react-native-device-info/commit/394c4bdcd270e1d8fac1011f098411f5c3591e5d

phillbaker commented 4 years ago

Thanks @mikehardy and @SaeedZhiany the project == rootProject change makes sense to me.

The compile -> implementation is a separate breaking change however. Today it only results in a warning - can we break that out separately?

mikehardy commented 4 years ago

I also like individual changes however, I'm not aware of compile -> implementation being breaking?

I just re-read related docs and I suppose if someone was using the module and needed transitivity then yes it's breaking. That would be unexpected by me but it is possible so I guess you're right :-)

SaeedZhiany commented 4 years ago

@phillbaker and @mikehardy

It's a rare case I think and I don't consider it as a breaking change, however, I can revert that changes and submit a separate PR for that. please tell me which one you want.

mikehardy commented 4 years ago

I think he already did ;-). rare -> can happen

I'm leaning more and more towards the react-native-webview policy ("This project follows semantic versioning. We do not hesitate to release breaking changes but they will be in a major version.")

SaeedZhiany commented 4 years ago

Ok then, I will revert the implementation changes in few seconds.

SaeedZhiany commented 4 years ago

@phillbaker @mikehardy

it's done.

SaeedZhiany commented 4 years ago

@phillbaker I submitted another PR for 'compile' to 'implementation' changes too, you can merge it whenever you decide to release another major version.

SaeedZhiany commented 4 years ago

Please also release a new version that includes this PR, I really need it in my project. thanks

phillbaker commented 4 years ago

Just published react-native-fingerprint-scanner@3.0.2