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.

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

Support type converters in .moor files #103

Closed
simolus3 opened this issue Aug 13, 2019 · 7 comments
Closed

Support type converters in .moor files #103

simolus3 opened this issue Aug 13, 2019 · 7 comments
Labels
enhancement New feature or request package-sqlparser Related to the sql parser

Comments

@simolus3
Copy link
Owner

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 #64 (comment)

@simolus3 simolus3 added enhancement New feature or request package-sqlparser Related to the sql parser labels Aug 13, 2019
@simolus3
Copy link
Owner Author

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
Copy link
Contributor

Mike278 commented Aug 13, 2019

That would be pretty slick!

@simolus3 simolus3 changed the title Support type conveters in .moor files Support type converters in .moor files Aug 19, 2019
@simolus3
Copy link
Owner Author

simolus3 commented Oct 9, 2019

AFAIK the build package doesn't support this at the moment, but dart-lang/build#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.

@simolus3 simolus3 added the blocked Work on this is blocked by external factors label Oct 9, 2019
@Mike278
Copy link
Contributor

Mike278 commented Oct 10, 2019

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 simolus3 removed the blocked Work on this is blocked by external factors label Nov 15, 2019
@simolus3
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package-sqlparser Related to the sql parser
Projects
None yet
Development

No branches or pull requests

3 participants