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

store.createRecord should be async? #5247

Closed
rwjblue opened this issue Nov 1, 2017 · 2 comments · Fixed by #5415
Closed

store.createRecord should be async? #5247

rwjblue opened this issue Nov 1, 2017 · 2 comments · Fixed by #5415
Assignees

Comments

@rwjblue
Copy link
Member

rwjblue commented Nov 1, 2017

While working on ember-qunit-codemod (which converts existing QUnit tests to the structure proposed in emberjs/rfcs#232) I have ran into something a bit surprising to me around store.createRecord(...) and I think it may be something that we should address (or at least discuss).

Currently, ember-data uses batching in a few locations:

To me, this effectively means that anything that runs through these paths is actually async (which roughly seems like "everything"?) and probably should return a promise.


Concretely, the issues I'm facing in testing land are:

  • store.createRecord(...) must be wrapped in a Ember.run to avoid triggering the auto-run assertion, in the prior moduleForModel API this was hidden from users behind this.subject(), but the new API is specifically intended to remove these extra "test only API's" and instead have users do what they would actually do in normal app code

  • calling run(() => store.createRecord(...)) in test behaves significantly different than what most users would do in "normal" app code (store.createRecord(...))

@bmac
Copy link
Member

bmac commented Nov 4, 2017

We talked about this at a recent Ember Data core meeting and it was agreed that createRecord should be made sync. We suspect the performance improvements with deferring relationship and record array updates don't make a big of an impact for createRecord then they do with pushing many records from the server.

@rwjblue
Copy link
Member Author

rwjblue commented Nov 5, 2017

👍 thank you for summarizing

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 a pull request may close this issue.

3 participants