simolus3 / drift

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

Support type converters in .moor files #103

Closed simolus3 closed 4 years ago

simolus3 commented 5 years ago

This is a great feature! I like being able to have table columns automatically mapped to enums. Any thoughts about adding support for this in .moor files? (#85)

Originally posted by @Mike278 in https://github.com/simolus3/moor/issues/64#issuecomment-520631511

simolus3 commented 5 years ago

Felt like this deserves its own issue because it requires some substantial changes.

I think the best way to use type mappers on columns is to extend the grammar, which is also what sqldelight is doing: https://github.com/square/sqldelight/blob/7db97c86ae0c83fe128bb9298fbf0ca3d15fbd2d/sqldelight-compiler/src/main/grammars/sqldelight.bnf#L51 Maybe we could have something like

CREATE TABLE users (
  preferences TEXT MAPPED BY `const CustomConverter()` NOT NULL
)

With that approach, we'd then have to parse the const CustomConverter() expression and use the Dart analyzer to figure out what type it is. Specifically, the CustomConverter extends TypeConverter<A,B>, and we need to know what A is. That A is the type of the field we need to generate. However, as .moor files are independent from .dart files, we would need to have some sort of import mechanism so that the analyzer knows about the CustomConverter, maybe it could look like:

import 'converters.dart';

CREATE TABLE users (
  preferences TEXT MAPPED BY `const CustomConverter()` NOT NULL
)

I've also never used the analyzer to parse and analyze segments of Dart code, it will be interesting to see how that works. I think angular is doing something similar with expressions in html files, I'll take a look at how they're doing it.

Mike278 commented 5 years ago

That would be pretty slick!

simolus3 commented 5 years ago

AFAIK the build package doesn't support this at the moment, but https://github.com/dart-lang/build/pull/2480 would help. The problem is that we don't get to resolve arbitrary Dart files yet. As soon as we're able to do that, we can generate a temporary, fake Dart file that contains the constructor invocation of the converter, so something like

// temp$0.dart
import 'converters.dart';

var converter = const CustomConverter();

for the example above. We should then be able to read the type by looking at the type of the declared "converter" field.

Mike278 commented 5 years ago

Darn that's too bad, although it looks like your PR is on track to be merged!

I tried messing around with Uri.dataFromString's ability to run dart code from a string in an isolate, but it doesn't look like you can send a Type across isolates:

import 'dart:isolate';

class TypeConverter<A, B> {}
class Preferences {}
class PreferenceConverter extends TypeConverter<Preferences, String> {}

void main() async {
  final typeConverterClass = 'PreferenceConverter';
  final uri = Uri.dataFromString(
    '''
    import "dart:isolate";
    import 'package:converter_test/main.dart';
    import "dart:mirrors";

    void main(_, SendPort port) {
      final Type type = reflectClass($typeConverterClass).superclass.typeArguments.first.reflectedType;
      print(type);
      port.send(type);
    }

    ''',
    mimeType: 'application/dart',
  );

  final port = ReceivePort();
  await Isolate.spawnUri(uri, [], port.sendPort);
  final Type type = await port.first;
  print(type);
}

// prints:
// Preferences
// Unhandled exception:
// Invalid argument(s): Illegal argument in isolate message : (object is a regular Dart Instance)
simolus3 commented 4 years ago

Looks like we're unblocked, but getting the next release ready is more important to me at the moment. The parser can handle this already, so hopefully it should be straightforward to implement after that.

simolus3 commented 4 years ago

Sorry for the long delay on this, but finally there is some good news. This was still blocked because there's no analyzer_plugin version that supports the latest analyzer, even though they're maintained by the same team. There still isn't, but I just uploaded a fork of that package which supports the latest analyzer, which means that type converters in moor files are finally possible.

This should work on develop now, and the next moor_generator release will support this feature. The syntax for type converters is as follows:

import 'converter.dart'; -- import the Dart file declaring the type converter

CREATE TABLE config (
    config_key TEXT not null primary key,
    config_value TEXT,
    sync_state INTEGER MAPPED BY `const SyncTypeConverter()`
) AS "Config";

One limitation: We can't have import statements in generated part of-files, so you'd need to import converter.dart into the Dart file where you have the database class.

Thanks for the patience!

kuhnroyal commented 4 years ago

Works on develop:

dependency_overrides:
  moor_generator:
    git:
      url: https://github.com/simolus3/moor.git
      path: moor_generator
      ref: develop
  sqlparser:
    git:
      url: https://github.com/simolus3/moor.git
      path: sqlparser
      ref: develop

The missing imports are a minor annoyance, it's not only the converter that needs to be imported but also the Dart type if it is not any default type.

One suggestion, add some field(s) to @UseMoor where you need to list the used converters and return types. This forces the imports and maybe even allows to validate some stuff during generation.