Decoupling the generated <Class>ToColumns mixin from the Class

[HeftyCoder]

I've been generally using the <Class>ToColumns mixin for my inserts and updates instead of a companion class, since I find it useful to simply pass all the data (if some data is the same, no harm done).

In the process of cleaning up the project, I created interfaces for my Database, to abstract away its implementation. I then realized that the <Class>ToColumns mixin is strongly coupled with the custom class itself. In light of this, might I suggest adding another static method in the generated companion class? For example:

class User {
    final int id;
    final String name;

    const User({required this.id, required this.name});
}

would result in

final user = User(id: 23, name: 'George');
final insertable = UsersCompanion.from(user);

[simolus3]

I like this idea and I've started implementing it, but I've realized halfway through that it doesn't work too well.

One thing we deliberately accept for custom row classes is to make the class more generic than data in the row (we need the ability to map rows to row classes, but not necessarily the other way around). So for instance, your users table could have additional columns that aren't mentioned in class User, and drift wouldn't complain because it's possible to map rows into User by just dropping unnecessary values.

The to-columns mixin avoids this issue by declaring all necessary fields as abstract getters, so you get a compilation error if you apply it on incompatible classes. With a UsersCompanion.from method, the generated code would necessarily be invalid for those constructs.

[HeftyCoder]

With a UsersCompanion.from method, the generated code would necessarily be invalid for those constructs.

Should this be a big concern, though? Be it a custom-class or a Drift-generated one, isn't the utility of a UsersCompanion.from method valuable? At the very least, for the class that is associated to the table itself, and not some other query that uses another custom class with partial rows (which I suspect wouldn't work with inserts/updates anyway).

While I can see value in using a User class that does not have all the rows, I also find it a bit of an anti-pattern, when that class is meant to be the main table class.

The to-columns mixin can still exist. This change should only enhance the current functionality.

As it is, Drift users that want to have their custom classes de-coupled from Drift have to create their own Insertable (or through a Companion) manually for every table/class. I believe it worth adding the functionality, and leaving it up to the user whether they want to be sure that all necessary fields are included by using the to-columns mixin, or directly use the from method.

---

Another possible solution would be to generate a UserProxy class that implements User through a User object. That way a user could have full control doing something like:

/// Generated Proxy class
class UserProxy implements User {
    final User user;
    const UserProxy(this.user);
    
    // Implementation through user field....
}

/// Insertable proxy that utilizes the UsersToColumns mixin
class InsertableUserProxy extends UserProxy with UsersToColumns { ... }

But I can see this being a lot of work and kind of verbose. If only Dart had an easy way to create a proxy class...

[simolus3]

I still agree that UsersCompanion.from would be useful, but I'm afraid generating it unconditionally may be a breaking change due to code breaking for incomplete row classes (even though that's quite odd).

One thing that may serve as a workaround is to pass generateInsertable:true on the @UseRowClass annotation. This is an opt-in which will make drift generate an extension on User to convert those instances into an Insertable<User>, so you could use that to turn instances into values to insert while still having the actual User class decoupled from this.

So maybe this is just a question of making that option more visible, or enabling it by default for compatible classes (but then I'm worried it may be tricky to debug when it may be missing).

[HeftyCoder]

I'm sorry, I am probably missing something. How is code going to break for incomplete row classes? Couldn't the generation of UsersCompanion.from be a shortcut for using the default UsersCompanion constructor, with all the non-detected values as absent? It's more of an extra utility, so that users of Drift don't have to manually recreate an insert/update from a custom class. Is there any worry that the custom class might not be defined properly by the user, hence they might make more mistakes than when they manually use the companion class?

I find the extensions quite as useful, if not more than a UsersCompanion.from. An extension is more appropriate for what I'm describing, as it could potentially allow for multiple custom classes without cluttering the companion class.