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.

Sign up for GitHub

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

Upcoming breaking changes in 3.1 canary 6 onwards. #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Upcoming breaking changes in 3.1 canary 6 onwards. #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 35 additions & 35 deletions app/src/main/java/com/github/nitrico/lastadapter_sample/ui/KotlinListFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,39 @@ import com.github.nitrico.lastadapter_sample.databinding.*
class KotlinListFragment : ListFragment() {

private val typeHeader = Type<ItemHeaderBinding>(R.layout.item_header)
.onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
.onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }

private val typeHeaderFirst = Type<ItemHeaderFirstBinding>(R.layout.item_header_first)
.onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
.onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }

private val typePoint = Type<ItemPointBinding>(R.layout.item_point)
.onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
.onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }

private val typeCar = Type<ItemCarBinding>(R.layout.item_car)
.onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
.onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }

private val typePerson = Type<ItemPersonBinding>(R.layout.item_person)
.onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
.onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }


override fun onActivityCreated(savedInstanceState: Bundle?) {
Expand Down Expand Up @@ -73,18 +73,18 @@ class KotlinListFragment : ListFragment() {
.map<Header, ItemHeaderBinding>(R.layout.item_header)
.map<Point>(typePoint)
.map<Car>(Type<ItemCarBinding>(R.layout.item_car)
.onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
.onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
.onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
.onRecycle { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
.onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
.onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }
)
.map<Person, ItemPersonBinding>(R.layout.item_person) {
onCreate { println("Created ${it.binding.item} at #${it.adapterPosition}") }
onBind { println("Bound ${it.binding.item} at #${it.adapterPosition}") }
onRecycle { println("Recycled ${it.binding.item} at #${it.adapterPosition}") }
onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding.item}") }
onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding.item}") }
onCreate { println("Created ${it.binding?.item} at #${it.adapterPosition}") }
onBind { println("Bound ${it.binding?.item} at #${it.adapterPosition}") }
onRecycle { println("Recycled ${it.binding?.item} at #${it.adapterPosition}") }
onClick { activity.toast("Clicked #${it.adapterPosition}: ${it.binding?.item}") }
onLongClick { activity.toast("Long-clicked #${it.adapterPosition}: ${it.binding?.item}") }
}
.into(list)
}
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ buildscript {
sdkMin : 14,
sdkTarget : 27,
buildTools : '27.0.1',
gradlePlugin : '3.0.1',
kotlin : '1.2.0',
gradlePlugin : '3.1.0-alpha06',
kotlin : '1.2.10',
support : '27.0.2'
]
repositories {
Expand Down
1 change: 1 addition & 0 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
# org.gradle.parallel=true

org.gradle.jvmargs=-Xmx2048m
android.databinding.enableV2=true
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-4.3.1-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.4-all.zip
12 changes: 12 additions & 0 deletions lastadapter/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
plugins {
id 'com.android.library'
id 'kotlin-android'
id 'kotlin-kapt'
}

android {
Expand All @@ -10,7 +11,18 @@ android {
dataBinding.enabled true
}

afterEvaluate {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a workaround as suggested by Yigit Boyar here, and actual change here

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed anymore, at least in 3.1 beta 1

android.libraryVariants.all {
def name = it.name.capitalize()
tasks["kapt${name}Kotlin"].dependsOn("transformDataBindingWithDataBindingMergeArtifactsFor${name}")

}
}


dependencies {
//Data binding
kapt "com.android.databinding:compiler:$versions.gradlePlugin"
compile "com.android.support:recyclerview-v7:$versions.support"
compile "org.jetbrains.kotlin:kotlin-stdlib:$versions.kotlin"
}
Expand Down
2 changes: 1 addition & 1 deletion lastadapter/src/main/java/com/github/nitrico/lastadapter/Holder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. I am awaiting official confirmation in release notes
  2. At work I can still see Databinding.inflate() retirns Binding! class. Which can be null from Java if not from kotlin.
  3. This comment increases confusion a lot. Coming from DataBindingUtil.java 3.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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@PrashamTrivedi PrashamTrivedi Jan 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optionally throw an exception

I am against throwing an exception in this case.

Reason(s)

  1. 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.

  2. 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),

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

internal var created = false
}
10 changes: 5 additions & 5 deletions lastadapter/src/main/java/com/github/nitrico/lastadapter/LastAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Author

@PrashamTrivedi PrashamTrivedi Dec 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it lateinit as I felt the real nature of this is late initialization, and won't be nullable ever after.


private val map = mutableMapOf<Class<*>, BaseType>()
private var layoutHandler: LayoutHandler? = null
Expand Down Expand Up @@ -90,7 +90,7 @@ class LastAdapter(private val list: List<Any>,
override fun onCreateViewHolder(view: ViewGroup, viewType: Int): Holder<ViewDataBinding> {
val binding = DataBindingUtil.inflate<ViewDataBinding>(inflater, viewType, view, false)
val holder = Holder(binding)
binding.addOnRebindCallback(object : OnRebindCallback<ViewDataBinding>() {
binding?.addOnRebindCallback(object : OnRebindCallback<ViewDataBinding>() {
override fun onPreBind(binding: ViewDataBinding) = recyclerView?.isComputingLayout ?: false
override fun onCanceled(binding: ViewDataBinding) {
if (recyclerView?.isComputingLayout ?: true) {
Expand All @@ -107,8 +107,8 @@ class LastAdapter(private val list: List<Any>,

override fun onBindViewHolder(holder: Holder<ViewDataBinding>, position: Int) {
val type = getType(position)!!
holder.binding.setVariable(getVariable(type), list[position])
holder.binding.executePendingBindings()
holder.binding?.setVariable(getVariable(type), list[position])
holder.binding?.executePendingBindings()
@Suppress("UNCHECKED_CAST")
if (type is AbsType<*>) {
if (!holder.created) {
Expand All @@ -120,7 +120,7 @@ class LastAdapter(private val list: List<Any>,

override fun onBindViewHolder(holder: Holder<ViewDataBinding>, position: Int, payloads: List<Any>) {
if (isForDataBinding(payloads)) {
holder.binding.executePendingBindings()
holder.binding?.executePendingBindings()
} else {
super.onBindViewHolder(holder, position, payloads)
}
Expand Down