Upcoming breaking changes in 3.1 canary 6 onwards. #38
Conversation
Databinding v2
| @@ -10,7 +11,18 @@ android { | |||
| dataBinding.enabled true | |||
| } | |||
|
|
|||
| afterEvaluate { | |||
There was a problem hiding this comment.
this is not needed anymore, at least in 3.1 beta 1
| @@ -35,7 +35,7 @@ class LastAdapter(private val list: List<Any>, | |||
| private val DATA_INVALIDATION = Any() | |||
| private val callback = ObservableListCallback(this) | |||
| private var recyclerView: RecyclerView? = null | |||
| private var inflater: LayoutInflater? = null | |||
| private lateinit var inflater: LayoutInflater | |||
There was a problem hiding this comment.
I made it lateinit as I felt the real nature of this is late initialization, and won't be nullable ever after.
|
Wow! Thank you so much for all the information! I'll keep all that in mind when the final version is released. :) |
| @@ -19,6 +19,6 @@ package com.github.nitrico.lastadapter | |||
| import android.databinding.ViewDataBinding | |||
| import android.support.v7.widget.RecyclerView | |||
|
|
|||
| open class Holder<B : ViewDataBinding>(val binding: B) : RecyclerView.ViewHolder(binding.root) { | |||
| open class Holder<B : ViewDataBinding>(val binding: B?) : RecyclerView.ViewHolder(binding?.root) { | |||
There was a problem hiding this comment.
the nullable binding variable will break a lot of existing code. You can/should even remove it as DataBindingUtil.inflate doesn't return a nullable binding (anymore?)
There was a problem hiding this comment.
They should've reverted this change. I don't have the link handy but in one of the issue in issue tracker Yigit Boyar mentioned that Databinding is supposed to be null in case the layout is not binding layout. And this was Working as intended type of thing,
At this point
- I am awaiting official confirmation in release notes
- At work I can still see Databinding.inflate() retirns Binding! class. Which can be null from Java if not from kotlin.
- This comment increases confusion a lot. Coming from
DataBindingUtil.java3.1.0beta1.
// @Nullable don't annotate with Nullable. It is unlikely to be null and makes using it from
// kotlin really ugly. We cannot make it NonNull w/o breaking backward compatibility.I will wait for official release notes around databinding changes before deciding to proceed on this change.
There was a problem hiding this comment.
Sure, makes sense!
Even if it'll be nullable we might work around that by checking for null before creating the Holder-instance and optionally throw an exception if it the binding is null. That way we wouldn't break backward compatibility with this library.
There was a problem hiding this comment.
optionally throw an exception
I am against throwing an exception in this case.
Reason(s)
-
Mostly this lib is being used with normal databinding, which means users have to handle binding nullability in places where this library is not used.
-
Most thrown exception is swallowed without having any actions. Based on this I fear throwing exception won't change anything except adding try catch block, mostly to log exceptions. (Even we do this in 80% cases)
break backward compatibility with this library.
With point 1 above, I feel backword compatibility is already broken. (Esp backward compatibility from databinding v2 to v1)
Anyways, I don't mind if the flow goes either way (Having nullable bindings or throwing exception),
There was a problem hiding this comment.
I agree. Let's see - maybe the google devs decide to handle this case (the user tries to load a non-bindable layout) themselves to sort this out.
There was a problem hiding this comment.
Update on null-ability on databinding, Here is the issue we reported w.r.t. nullability, based on their response what they did is not to force null-ability when we're using kotlin. I don't know how this changes our discussion but I am putting this as reference link.
Android studio 3.1 Canary 6 introduces Databinding compiler which introduces new changes which are breaking for library consumers. As per Release Notes binding code is generated and packaged in AAR, and dependent modules are no longer re-generating library bindings.
Also as per documentation this changes are backward incompatible, means consumers of Databinding v2 can't use library or code generated against Databinding v1.
Another change as stated here data binding inflation methods (
inflateandsetContentViewetc...) will return nullable bindings (Null in case of layout is not binding layout). Which is breaking change for consumers.This pull request facilitates both above stated changes, but I suggest be cautious as the changes are for alpha release of plugin.