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

Improve nextId() generator #64

Closed
wants to merge 1 commit into from

Conversation

pronebird
Copy link

Improves id generator to provide means to override default sequence.

I wasn't able to write tests, not sure how to mock the backend etc.

One of the tests would be to create model with arbitrary id and see if sessionData.nextId changes.

Also, in this change nextId() will always bump the counter, which is fine since it's not called anywhere else but within Model.

@pronebird pronebird mentioned this pull request Nov 20, 2016
@pronebird
Copy link
Author

pronebird commented Nov 20, 2016

Example usage with uuid:

import uuid from 'uuid';

class MyModel extends Model {

  static nextId() {
    return uuid.v4();
  }

  static advanceNextId(id) {
    // uuids are not deterministic
    // no-op
  }
}

@pronebird
Copy link
Author

I am testing this and it seems to work fine in on of my apps.

@tommikaikkonen
Copy link
Collaborator

@pronebird I really appreciate the effort you put here. I ended up implementing this a little differently in #62 . For the pretty common UUID case, this should work:

class MyModel extends Model { }
MyModel.idAttribute = 'id';
MyModel.fields = {
    id: attr({ getDefault: uuid.v4 }), // might need uuid.v4.bind(uuid)
};

@pronebird
Copy link
Author

pronebird commented Dec 12, 2016

@tommikaikkonen That's great! Thanks for your efforts too! Did you solve the problem with id sequencer for non-deterministic IDs such as UUID? Previous implementation tended to +1 UUIDs :)

@pronebird
Copy link
Author

See comments above.

@pronebird pronebird closed this Apr 24, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants