-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix setting ID on Record Data from DS.Model #6775
Conversation
Asset Size Report for 683ac50 EmberData increased by 91.0 B uncompressed but decreased by 107.0 B compressedWarningsThe uncompressed size of the package @ember-data/store has increased by 56.0 B. Changeset
Full Asset Analysis
|
683ac50
to
341d4df
Compare
Asset Size Report for 341d4df EmberData increased by 92.0 B uncompressed but decreased by 91.0 B compressedWarningsThe uncompressed size of the package @ember-data/store has increased by 56.0 B. Changeset
Full Asset Analysis
|
341d4df
to
32cec61
Compare
Asset Size Report for 32cec61 EmberData increased by 92.0 B uncompressed but decreased by 91.0 B compressedWarningsThe uncompressed size of the package @ember-data/store has increased by 56.0 B. Changeset
Full Asset Analysis
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TS 👍
@@ -1359,6 +1359,10 @@ export default class InternalModel { | |||
|
|||
if (didChange && id !== null) { | |||
this.store.setRecordId(this.modelName, id, this.clientId); | |||
// internal set of ID to get it to RecordData from DS.Model | |||
if (this._recordData.__setId) { | |||
this._recordData.__setId(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may want to leave yourself a note to use optional chaining here when TS 3.7 is safe to adopt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, want to leave the check more loud for the reader/explicit in this case
@@ -372,6 +372,13 @@ module('unit/model - Model', function(hooks) { | |||
assert.equal(idChange, 0); | |||
person._internalModel.setId('john'); | |||
assert.equal(idChange, 1); | |||
let recordData = recordDataFor(person); | |||
assert.equal( | |||
recordData.getResourceIdentifier().id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch
No description provided.