billysometimes / rethinkdb

dart driver for rethinkDB
MIT License
37 stars 19 forks source link

Support of Dart 2.0.0 #63

Open alexd1971 opened 6 years ago

alexd1971 commented 6 years ago

Are you going to upgrade to Dart 2.0.0 support?

billysometimes commented 6 years ago

Sure! I think the changes will be minimal so it shouldn't take too long.

billysometimes commented 6 years ago

Actually I guess this is more complicated than I had previously thought. Dart 2.0.0 has greatly changed the way noSuchMethod works. Previously we could abuse it to allow for varargs, for example getAll we could do any of the following: r.getAll('moe', {'index': 'first_name'}) r.getAll('moe', 'larry, {'index': 'first_name'}) r.getAll('moe', 'larry, 'curly', {'index': 'first_name'})

Only the first example matched the method signature, the next two would be invoked through noSuchMethod. It seems that with dart 2.0, we can no longer do this.

I could still update the driver, but when writing with dart 2.0 you would not be able to follow the API, you'd have to rely on the args method a great deal in order to make the every method match it's signature:

r.getAll(r.args(['moe', 'larry, 'curly']), {'index': 'first_name'}) r.branch(hour > 12, 'Good morning', r.args([hour < 17, 'Good afternoon', 'Good evening']))

I think this really hurts readability, but I am open to suggestions and discussion.

pafik13 commented 6 years ago

@billysometimes, Could you make a branch with 2.0 version?

pafik13 commented 6 years ago

@billysometimes, Thank you for the branch. I try to connect and get "Server dropped connection with message: Wrong password" everytime. I tested it with default server configuration. If I use 1.24.3 sdk I haven't connection problems. What am I doing wrong?

pafik13 commented 6 years ago

I found that new PBDKF package working wrong. I update old package and make PR here

thosakwe commented 6 years ago

LMK if you need any help rolling this... Without Dart 2 support, I can't update package:angel_rethink, so I'm more than willing to send a PR.

thosakwe commented 6 years ago

Ping

billysometimes commented 6 years ago

Sorry about the delay. @pafik13 submitted a PR to fix the PBKDF2 package, but they haven't published a new package to pub (https://pub.dartlang.org/packages/pbkdf2).

I have updated the dart-2.0 branch to pull the package from github, so you could change your dependencies to:

  rethinkdb_driver:
    git:
      url: git://github.com/billysometimes/rethinkdb.git
      ref: dart-2.0

but quite honestly I don't know if we should publish a 2.0 compatible package until we figure out what a Rethinkdb Query with varargs should look like in a language that does not have varargs.

I'd also like to find out why the more up-to-date package:password_hash PBKDF2 utils seem incompatible with this driver.

And lastly I'm not certain it's worth updating a driver for a dead database written in a mostly dead language.

thosakwe commented 6 years ago
  1. I see what you mean about them not having uploaded a new PBKDF2 package... I’ll try to email them.
  2. Dart is far from dead - if anything, it’s more alive than ever.
  3. If you are ever looking for another maintainer, or just contributions in general, I’m more than happy to do it. I guess it being “worth it” is a value judgment. To me, updating it is worth it, so whatever assistance you need, I’ll provide. On Sat, Nov 10, 2018 at 6:51 PM billy notifications@github.com wrote:

Sorry about the delay. @pafik13 https://github.com/pafik13 submitted a PR to fix the PBKDF2 package, but they haven't published a new package to pub (https://pub.dartlang.org/packages/pbkdf2).

I have updated the dart-2.0 branch to pull the package from github, so you could change your dependencies to:

rethinkdb_driver: git: url: git://github.com/billysometimes/rethinkdb.git ref: dart-2.0

but quite honestly I don't know if we should publish a 2.0 compatible package until we figure out what a Rethinkdb Query with varargs should look like in a language that does not have varargs.

I'd also like to find out why the more up-to-date package:password_hash PBKDF2 utils seem incompatible with this driver.

And lastly I'm not certain it's worth updating a driver for a dead database written in a mostly dead language.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/billysometimes/rethinkdb/issues/63#issuecomment-437631203, or mute the thread https://github.com/notifications/unsubscribe-auth/AJiKPF98ahg8gbS8M8Ef1W2-ZSOssq3Tks5ut2adgaJpZM4WcvAW .

ghost commented 5 years ago

flutter+rethink may be a great combo

rullyalves commented 5 years ago

Hello, I really liked the driver idea and would like to help in your migration to Dart 2.1, to be possible, I would like to start an immigration with you, I wanted to see this driver running on the aqueduct and on Flutter itself.

thosakwe commented 5 years ago

@rullyalves Angel framework has top-notch support for RethinkDB - that’s why I want to update this library.

marceloneppel commented 5 years ago

Hi @billysometimes! I implemented some of the fixes in the varargs. Could you check them? I'll try to implement the rest.

marceloneppel commented 5 years ago

There are still some calls to dart:mirrors that prevent the use of the library on Flutter. I'll try to fix this.

thosakwe commented 5 years ago

Thanks @billysometimes! I now have push access. @katutz Stay tuned, within the next few days, I'll work with you to land these changes and update this library for Dart 2.

marceloneppel commented 5 years ago

Anyone here knows if using the #foo directive would impact some type of minification on dart2js or in other dart environment?

@override
dynamic noSuchMethod(Invocation invocation) {
  return invocation.memberName == #foo
      ? Function.apply(
          baz, invocation.positionalArguments, invocation.namedArguments)
      : super.noSuchMethod(invocation);
}

If no, maybe we could simplify some parts of the code that I commited. Reference link: https://www.dartlang.org/articles/language/emulating-functions

marceloneppel commented 5 years ago

I added a pull request about the ClosureMirror removal. It is useful to make the driver works with Flutter. There is another mirror use, InstanceMirror, which can be removed with the lasts varargs fix, that still needs to be implemented.

marceloneppel commented 5 years ago

Any updates about my last PR, @billysometimes and @thosakwe?