Skip to content

Commit d4c48d5

Browse files
committed
[BUGFIX release beta canary] Fix Model lifecycle event deprecations
Related RFC: https://github.com/emberjs/rfcs/blob/master/text/0329-deprecated-ember-evented-in-ember-data.md Original PR: #6059 Deprecations were added for the following model lifecycle events `becameError`, `becameInvalid`, `didCreate`, `didDelete`, `didLoad`, `didUpdate`, `ready`, `rolledBack` in #6059. There was a discussion on the PR that suggested adding an additional check to make sure that that the deprecations were only triggered for instances of Ember Data's `Model` class #6059 (comment) This extra check was added but it accidentally was checking for `this` to be an instance of the `Model` class, but that can never be true since `this` will always be an instance of `InternalModel`: https://github.com/emberjs/data/blob/5e495394cbf67025be2622f604e67004d79bc21c/packages/store/addon/-private/system/model/internal-model.ts#L446-L455 Since we only want to check for these deprecations for instances of `Model`, I moved the deprecation logic to the `Model` class #6059 also included a mechanism for tracking deprecations that were logged per model class so we could prevent logging multiple deprecations for the same model class and deprecated lifecycle method pair: https://github.com/emberjs/data/blob/5e495394cbf67025be2622f604e67004d79bc21c/packages/store/addon/-private/system/model/internal-model.ts#L61-L77 This also fixes a bug with the deprecation tracking logic which was that we were never actually adding the logged deprecations to the `WeakMap` meant to keep track of them.
1 parent 5e49539 commit d4c48d5

File tree

4 files changed

+71
-46
lines changed

4 files changed

+71
-46
lines changed

.eslintrc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ module.exports = {
3636
heimdall: true,
3737
Map: false,
3838
WeakMap: true,
39+
Set: true,
3940
},
4041
env: {
4142
browser: true,

packages/-ember-data/tests/unit/model-test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,34 @@ module('unit/model - Model', function(hooks) {
665665
assert.equal(eventMethodArgs[1], 2);
666666
}
667667
);
668+
669+
testInDebug('defining record lifecycle event methods on a model class is deprecated', async function(assert) {
670+
class EngineerModel extends Model {
671+
becameError() {}
672+
becameInvalid() {}
673+
didCreate() {}
674+
didDelete() {}
675+
didLoad() {}
676+
didUpdate() {}
677+
ready() {}
678+
rolledBack() {}
679+
}
680+
681+
this.owner.register('model:engineer', EngineerModel);
682+
683+
let store = this.owner.lookup('service:store');
684+
685+
store.createRecord('engineer');
686+
687+
assert.expectDeprecation(/You defined a `becameError` method for model:engineer but lifecycle events/);
688+
assert.expectDeprecation(/You defined a `becameInvalid` method for model:engineer but lifecycle events/);
689+
assert.expectDeprecation(/You defined a `didCreate` method for model:engineer but lifecycle events/);
690+
assert.expectDeprecation(/You defined a `didDelete` method for model:engineer but lifecycle events/);
691+
assert.expectDeprecation(/You defined a `didLoad` method for model:engineer but lifecycle events/);
692+
assert.expectDeprecation(/You defined a `didUpdate` method for model:engineer but lifecycle events/);
693+
assert.expectDeprecation(/You defined a `ready` method for model:engineer but lifecycle events/);
694+
assert.expectDeprecation(/You defined a `rolledBack` method for model:engineer but lifecycle events/);
695+
});
668696
});
669697

670698
module('Reserved Props', function() {

packages/store/addon/-private/system/model/internal-model.ts

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -58,24 +58,6 @@ interface BelongsToMetaWrapper {
5858
modelName: string;
5959
}
6060

61-
let INSTANCE_DEPRECATIONS;
62-
let lookupDeprecations;
63-
64-
if (DEBUG) {
65-
INSTANCE_DEPRECATIONS = new WeakMap();
66-
67-
lookupDeprecations = function lookupInstanceDrecations(instance) {
68-
let deprecations = INSTANCE_DEPRECATIONS.get(instance);
69-
70-
if (!deprecations) {
71-
deprecations = {};
72-
INSTANCE_DEPRECATIONS.set(instance, deprecations);
73-
}
74-
75-
return deprecations;
76-
};
77-
}
78-
7961
/*
8062
The TransitionChainMap caches the `state.enters`, `state.setups`, and final state reached
8163
when transitioning from one state to another, so that future transitions can replay the
@@ -430,31 +412,6 @@ export default class InternalModel {
430412

431413
this._record = store._modelFactoryFor(this.modelName).create(createOptions);
432414
setRecordIdentifier(this._record, this.identifier);
433-
if (DEBUG) {
434-
let klass = this._record.constructor;
435-
let deprecations = lookupDeprecations(klass);
436-
[
437-
'becameError',
438-
'becameInvalid',
439-
'didCreate',
440-
'didDelete',
441-
'didLoad',
442-
'didUpdate',
443-
'ready',
444-
'rolledBack',
445-
].forEach(methodName => {
446-
if (this instanceof Model && typeof this._record[methodName] === 'function') {
447-
deprecate(
448-
`Attempted to define ${methodName} on ${this._record.modelName}#${this._record.id}`,
449-
deprecations[methodName],
450-
{
451-
id: 'ember-data:record-lifecycle-event-methods',
452-
until: '4.0',
453-
}
454-
);
455-
}
456-
});
457-
}
458415
}
459416
}
460417
this._triggerDeferredTriggers();

packages/store/addon/-private/system/model/model.js

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,13 +1363,34 @@ if (DEBUG) {
13631363
return isBasicDesc(instanceDesc) && lookupDescriptor(obj.constructor, keyName) === null;
13641364
};
13651365

1366+
const INSTANCE_DEPRECATIONS = new WeakMap();
1367+
const DEPRECATED_LIFECYCLE_EVENT_METHODS = [
1368+
'becameError',
1369+
'becameInvalid',
1370+
'didCreate',
1371+
'didDelete',
1372+
'didLoad',
1373+
'didUpdate',
1374+
'ready',
1375+
'rolledBack',
1376+
];
1377+
1378+
let lookupDeprecations = function lookupInstanceDeprecations(instance) {
1379+
let deprecations = INSTANCE_DEPRECATIONS.get(instance);
1380+
1381+
if (!deprecations) {
1382+
deprecations = new Set();
1383+
INSTANCE_DEPRECATIONS.set(instance, deprecations);
1384+
}
1385+
1386+
return deprecations;
1387+
};
1388+
13661389
Model.reopen({
13671390
init() {
13681391
this._super(...arguments);
13691392

1370-
if (DEBUG) {
1371-
this._getDeprecatedEventedInfo = () => `${this._internalModel.modelName}#${this.id}`;
1372-
}
1393+
this._getDeprecatedEventedInfo = () => `${this._internalModel.modelName}#${this.id}`;
13731394

13741395
if (!isDefaultEmptyDescriptor(this, '_internalModel') || !(this._internalModel instanceof InternalModel)) {
13751396
throw new Error(
@@ -1393,6 +1414,24 @@ if (DEBUG) {
13931414
`You may not set 'id' as an attribute on your model. Please remove any lines that look like: \`id: DS.attr('<type>')\` from ${this.constructor.toString()}`
13941415
);
13951416
}
1417+
1418+
let lifecycleDeprecations = lookupDeprecations(this.constructor);
1419+
1420+
DEPRECATED_LIFECYCLE_EVENT_METHODS.forEach(methodName => {
1421+
if (typeof this[methodName] === 'function' && !lifecycleDeprecations.has(methodName)) {
1422+
deprecate(
1423+
`You defined a \`${methodName}\` method for ${this.constructor.toString()} but lifecycle events for models have been deprecated.`,
1424+
false,
1425+
{
1426+
id: 'ember-data:record-lifecycle-event-methods',
1427+
until: '4.0',
1428+
url: 'https://deprecations.emberjs.com/ember-data/v3.x#toc_record-lifecycle-event-methods',
1429+
}
1430+
);
1431+
1432+
lifecycleDeprecations.add(methodName);
1433+
}
1434+
});
13961435
},
13971436
});
13981437
}

0 commit comments

Comments
 (0)