simolus3 / drift

Drift is an easy to use, reactive, typesafe persistence library for Dart & Flutter.
https://drift.simonbinder.eu/
MIT License
2.55k stars 364 forks source link

DevTools extension replace dependency from `db_viewer` to `drift_db_viewer` #3185

Closed FMorschel closed 3 days ago

FMorschel commented 2 weeks ago

I noticed that the extension only depends on db_viewer and not drift_db_viewer. I wonder why.

The https://pub.dev/packages/drift_db_viewer has even built-in support for where clauses which the db_viewer used inside the extension has not.

I'd like to ask to replace the dependency. But if there is a clear motive for not replacing them, please let me know.

simolus3 commented 2 weeks ago

I don't remember the exact reason to be honest - I think it was something about drift_db_viewer not exporting the DbViewerDatabase implementation directly and instead requiring the full drift database (which a RemoteDatabase isn't, but we can change that if needed).

So if you get drift_db_viewer working in the extension, then that's definitely better than the current solution. Maybe we can make our ViewerDatabase class extend the class in drift_db_viewer to inherit buildWhereClause.

FMorschel commented 2 weeks ago

I think I finally figured out what might work here. Let me know your thoughts.

There is the above PR for creating an interface like the following. This will shorten our work since we don't have to implement the full GeneratedDatabase interface:

abstract class DBHandler {
  Iterable<TableInfo<Table, dynamic>> get allTables;

  SqlTypes get typeMapping;

  Selectable<QueryRow> customSelect(
    String query, {
    Set<ResultSetImplementation<dynamic, dynamic>> readsFrom = const {},
    List<Variable<Object>> variables = const [],
  });

  Future<void> customStatement(String query, [List<dynamic>? args]);
}

I was wondering if we could actually make this interface something from drift (because of the function signatures) but I think if that is implemented there it might work anyway.

In any case, I'll try to make the current implementation fit that interface. Will keep you posted.