[DOC beta] Assert that both modelName and id are passed to peekRecord

[bantic]

I've seen folks get confused by forgetting that a modelName is a required param and then do something like store.peekRecord('my-id'). This assertion would point out that both values must be passed. Thanks for considering it. :)

[stefanpenner]

mind adding an expectAssertion test ?

[bantic]

sure thing

[bantic]

Updated. Also added an expectAssertion for the existing assertion that a model name (not a class) must be passed (e.g. when calling store.peekRecord(modelClass, id)).

[bmac]

@bantic could you use testInDebug instead of test for these tests. The assert gets stripped for the production build and is causing the CI to fail when we run tests in production mode.

import testInDebug from 'dummy/tests/helpers/test-in-debug';

[bantic]

@bmac Thanks for pointing that out. I made the change and updated this PR. I'll keep an eye on it to confirm the tests now pass.

[bantic]

The appveyor tests appear to have passed, but appveyor reports a timeout. Might just need a kick to re-run the tests; afaict everything should be passing now.

[pangratz]

Thanks a bunch @bantic!

[sumeetattree]

So this thing crept up on me today while upgrading from ember-data 2.16 to 3.1.

I am not sure if this is correct way to ensure the passing of a 2nd parameter to the method. I have a valid use case where I am fetching the record in an existing component. And the 2nd parameter may be absent null/undefined. In that case peekRecord should return null instead now it throws an Exception.

// some-component.js
...
colorId: undefined, // set on the component
fruit: computed('colorId', function() {
  // colorId can be null/undefined
  return this.get('store').peekRecord('fruit', this.get('colorId'));
}),

If you still want to keep the assertion maybe modify the peekRecord method to check for number of arguments instead of just checking if id is present?

@stefanpenner @bantic Thoughts?

[pangratz]

peekRecord without an id IMHO doesn't make sense and I would say this should be handled a layer up in your app code, so basically:

import { isNone } from '@ember/utils';

colorId: undefined, // set on the component

fruit: computed('colorId', function() {
  let colorId = this.get('colorId');

  if (!isNone(colorId)) {
    return this.get('store').peekRecord('fruit', colorId);
  }

  return null;
})

I would argue that this is much saner to reason about on what the expected behaviour of that computed property is.