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 of Dart 2.0.0 #63

Open
alexd1971 opened this issue Sep 6, 2018 · 18 comments
Open

Support of Dart 2.0.0 #63

alexd1971 opened this issue Sep 6, 2018 · 18 comments

Comments

@alexd1971
Copy link

Are you going to upgrade to Dart 2.0.0 support?

@billysometimes
Copy link
Owner

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

@billysometimes billysometimes self-assigned this Sep 7, 2018
@billysometimes
Copy link
Owner

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.

@billysometimes billysometimes removed their assignment Sep 11, 2018
@pafik13
Copy link

pafik13 commented Sep 12, 2018

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

@pafik13
Copy link

pafik13 commented Sep 16, 2018

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

pafik13 commented Sep 16, 2018

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

@thosakwe
Copy link
Collaborator

thosakwe commented Nov 3, 2018

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

thosakwe commented Nov 7, 2018

Ping

@billysometimes
Copy link
Owner

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

thosakwe commented Nov 11, 2018 via email

@ghost
Copy link

ghost commented Nov 21, 2018

flutter+rethink may be a great combo

@rullyalves
Copy link

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

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

@marceloneppel
Copy link

marceloneppel commented Nov 26, 2018

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

@marceloneppel
Copy link

marceloneppel commented Nov 27, 2018

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

@thosakwe
Copy link
Collaborator

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

marceloneppel commented Nov 28, 2018

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants