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

refactor: Native Model #7226

Merged
merged 13 commits into from
Apr 21, 2021
6 changes: 3 additions & 3 deletions .github/workflows/asset-size-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ jobs:
- name: Analyze ${{github.ref}} Assets (IE11)
run: |
node ./bin/asset-size-tracking/generate-analysis.js packages/-ember-data/dists/experiment-ie11 ./experiment-ie11-data.json
node ./bin/asset-size-tracking/print-analysis.js ./experiment-ie11-data.json > tmp/asset-sizes/experiment-analysis-ie11.txt
node ./bin/asset-size-tracking/print-analysis.js ./experiment-ie11-data.json -show > tmp/asset-sizes/experiment-analysis-ie11.txt
- name: Analyze ${{github.ref}} Assets
run: |
node ./bin/asset-size-tracking/generate-analysis.js packages/-ember-data/dists/experiment ./experiment-data.json
node ./bin/asset-size-tracking/print-analysis.js ./experiment-data.json > tmp/asset-sizes/experiment-analysis.txt
node ./bin/asset-size-tracking/print-analysis.js ./experiment-data.json -show > tmp/asset-sizes/experiment-analysis.txt
- name: Analyze ${{github.ref}} Assets
run: |
node ./bin/asset-size-tracking/generate-analysis.js packages/-ember-data/dists/experiment-no-rollup ./experiment-data-no-rollup.json
node ./bin/asset-size-tracking/print-analysis.js ./experiment-data-no-rollup.json > tmp/asset-sizes/experiment-analysis-no-rollup.txt
node ./bin/asset-size-tracking/print-analysis.js ./experiment-data-no-rollup.json -show > tmp/asset-sizes/experiment-analysis-no-rollup.txt
- name: Test Asset Sizes (IE11)
run: |
set -o pipefail
Expand Down
2 changes: 1 addition & 1 deletion packages/-ember-data/node-tests/docs/test-coverage.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ QUnit.module('Docs coverage', function(hooks) {

QUnit.module('modules', function() {
test('We have all expected modules', function(assert) {
assert.deepEqual(Object.keys(docs.modules), expected.modules, 'We have all modules');
assert.deepEqual(Object.keys(docs.modules).sort(), expected.modules, 'We have all modules');
});
});

Expand Down
7 changes: 3 additions & 4 deletions packages/-ember-data/node-tests/fixtures/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ module.exports = {
'@ember-data/adapter',
'@ember-data/canary-features',
'@ember-data/debug',
'@ember-data/model',
'@ember-data/store',
'@ember-data/deprecations',
'@ember-data/model',
'@ember-data/record-data',
'@ember-data/serializer'
'@ember-data/serializer',
'@ember-data/store',
],
classitems: [
'(private) @ember-data/adapter BuildURLMixin#_buildURL',
Expand All @@ -32,7 +32,6 @@ module.exports = {
'(private) @ember-data/model Model#_notifyProperties',
'(private) @ember-data/model Model#create',
'(private) @ember-data/model Model#currentState',
'(private) @ember-data/model Model#recordData',
'(private) @ember-data/model Model#send',
'(private) @ember-data/model Model#transitionTo',
'(private) @ember-data/model Model#trigger',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ module('integration/adapter/store-adapter - DS.Store and DS.Adapter integration
});
});

test('if a created record is marked as invalid by the server, it enters an error state', function(assert) {
test('if a created record is marked as invalid by the server, it enters an error state', async function(assert) {
let store = this.owner.lookup('service:store');
let adapter = store.adapterFor('application');
let Person = store.modelFor('person');
Expand Down Expand Up @@ -560,32 +560,30 @@ module('integration/adapter/store-adapter - DS.Store and DS.Adapter integration
let yehuda = store.createRecord('person', { id: 1, name: 'Yehuda Katz' });
// Wrap this in an Ember.run so that all chained async behavior is set up
// before flushing any scheduled behavior.
return run(function() {
return yehuda
.save()
.catch(error => {
assert.false(get(yehuda, 'isValid'), 'the record is invalid');
assert.ok(get(yehuda, 'errors.name'), 'The errors.name property exists');

set(yehuda, 'updatedAt', true);
assert.false(get(yehuda, 'isValid'), 'the record is still invalid');
try {
await yehuda.save();
} catch (e) {
assert.false(get(yehuda, 'isValid'), 'the record is invalid');
assert.ok(get(yehuda, 'errors.name'), 'The errors.name property exists');

set(yehuda, 'name', 'Brohuda Brokatz');
set(yehuda, 'updatedAt', true);
assert.false(get(yehuda, 'isValid'), 'the record is still invalid');

assert.true(get(yehuda, 'isValid'), 'the record is no longer invalid after changing');
assert.true(get(yehuda, 'hasDirtyAttributes'), 'the record has outstanding changes');
set(yehuda, 'name', 'Brohuda Brokatz');

assert.true(get(yehuda, 'isNew'), 'precond - record is still new');
assert.true(get(yehuda, 'isValid'), 'the record is no longer invalid after changing');
assert.true(get(yehuda, 'hasDirtyAttributes'), 'the record has outstanding changes');

return yehuda.save();
})
.then(person => {
assert.strictEqual(person, yehuda, 'The promise resolves with the saved record');
assert.true(get(yehuda, 'isNew'), 'precond - record is still new');

assert.true(get(yehuda, 'isValid'), 'record remains valid after committing');
assert.false(get(yehuda, 'isNew'), 'record is no longer new');
});
});
let person = await yehuda.save();

assert.strictEqual(person, yehuda, 'The promise resolves with the saved record');

assert.true(get(yehuda, 'isValid'), 'record remains valid after committing');
assert.false(get(yehuda, 'isNew'), 'record is no longer new');
}
});

test('allows errors on arbitrary properties on create', function(assert) {
Expand Down
26 changes: 18 additions & 8 deletions packages/-ember-data/tests/unit/model-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,21 +373,31 @@ module('unit/model - Model', function(hooks) {

test('ID mutation (complicated)', async function(assert) {
let idChange = 0;
let compChange = 0;
const OddPerson = Model.extend({
name: DSattr('string'),
idComputed: computed('id', function() {}),
idDidChange: observer('id', () => idChange++),
idComputed: computed('id', function() {
// we intentionally don't access the id here
return 'not-the-id:' + compChange++;
}),
idDidChange: observer('id', function() {
idChange++;
}),
});
this.owner.register('model:odd-person', OddPerson);

let person = store.createRecord('odd-person');
person.get('idComputed');
assert.equal(idChange, 0);
assert.strictEqual(person.get('idComputed'), 'not-the-id:0');
assert.equal(idChange, 0, 'we have had no changes initially');

assert.equal(person.get('id'), null, 'initial created model id should be null');
assert.equal(idChange, 0);
let personId = person.get('id');
assert.strictEqual(personId, null, 'initial created model id should be null');
assert.equal(idChange, 0, 'we should still have no id changes');

// simulate an update from the store or RecordData that doesn't
// go through the internalModelFactory
person._internalModel.setId('john');
assert.equal(idChange, 1);
assert.equal(idChange, 1, 'we should have one change after updating id');
let recordData = recordDataFor(person);
assert.equal(
recordData.getResourceIdentifier().id,
Expand Down Expand Up @@ -729,7 +739,7 @@ module('unit/model - Model', function(hooks) {

assert.expectAssertion(() => {
record.set('isLoaded', true);
}, /Cannot set read-only property "isLoaded"/);
}, /Cannot set property isLoaded of \[object Object\] which has only a getter/);
});

class NativePostWithInternalModel extends Model {
Expand Down
161 changes: 161 additions & 0 deletions packages/-ember-data/tests/unit/model/attr-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
import { module, test } from 'qunit';

import { setupTest } from 'ember-qunit';

import Model, { attr } from '@ember-data/model';

module('unit/model/attr | attr syntax', function(hooks) {
setupTest(hooks);

let store;
let owner;
hooks.beforeEach(function() {
owner = this.owner;
store = owner.lookup('service:store');
});

test('attr can be used with classic syntax', async function(assert) {
const User = Model.extend({
name: attr(),
nameWithTransform: attr('string'),
nameWithOptions: attr({}),
nameWithTransformAndOptions: attr('string', {}),
});

owner.register('model:user', User);

let UserModel = store.modelFor('user');
let attrs = UserModel.attributes;
assert.true(attrs.has('name'), 'We have the attr: name');
assert.true(attrs.has('nameWithTransform'), 'We have the attr: nameWithTransform');
assert.true(attrs.has('nameWithOptions'), 'We have the attr: nameWithOptions');
assert.true(attrs.has('nameWithTransformAndOptions'), 'We have the attr: nameWithTransformAndOptions');

let userRecord = store.push({
data: {
type: 'user',
id: '1',
attributes: {
name: 'Chris',
nameWithTransform: '@runspired',
nameWithOptions: 'Contributor',
nameWithTransformAndOptions: '@runspired contribution',
},
},
});

assert.strictEqual(userRecord.name, 'Chris', 'attr is correctly set: name');
assert.strictEqual(userRecord.nameWithTransform, '@runspired', 'attr is correctly set: nameWithTransform');
assert.strictEqual(userRecord.nameWithOptions, 'Contributor', 'attr is correctly set: nameWithOptions');
assert.strictEqual(
userRecord.nameWithTransformAndOptions,
'@runspired contribution',
'attr is correctly set: nameWithTransformAndOptions'
);
});

test('attr can be used with native syntax decorator style', async function(assert) {
class User extends Model {
@attr() name;
@attr('string') nameWithTransform;
@attr({}) nameWithOptions;
@attr('string', {}) nameWithTransformAndOptions;
}

owner.register('model:user', User);

let UserModel = store.modelFor('user');
let attrs = UserModel.attributes;
assert.true(attrs.has('name'), 'We have the attr: name');
assert.true(attrs.has('nameWithTransform'), 'We have the attr: nameWithTransform');
assert.true(attrs.has('nameWithOptions'), 'We have the attr: nameWithOptions');
assert.true(attrs.has('nameWithTransformAndOptions'), 'We have the attr: nameWithTransformAndOptions');

let userRecord = store.push({
data: {
type: 'user',
id: '1',
attributes: {
name: 'Chris',
nameWithTransform: '@runspired',
nameWithOptions: 'Contributor',
nameWithTransformAndOptions: '@runspired contribution',
},
},
});

assert.strictEqual(userRecord.name, 'Chris', 'attr is correctly set: name');
assert.strictEqual(userRecord.nameWithTransform, '@runspired', 'attr is correctly set: nameWithTransform');
assert.strictEqual(userRecord.nameWithOptions, 'Contributor', 'attr is correctly set: nameWithOptions');
assert.strictEqual(
userRecord.nameWithTransformAndOptions,
'@runspired contribution',
'attr is correctly set: nameWithTransformAndOptions'
);
});

test('attr cannot be used with native syntax prop style', async function(assert) {
// TODO it would be nice if this syntax error'd but it currently doesn't
class User extends Model {
name = attr();
nameWithTransform = attr('string');
nameWithOptions = attr({});
nameWithTransformAndOptions = attr('string', {});
}

owner.register('model:user', User);

let UserModel = store.modelFor('user');
let attrs = UserModel.attributes;
assert.false(attrs.has('name'), 'We have the attr: name');
assert.false(attrs.has('nameWithTransform'), 'We have the attr: nameWithTransform');
assert.false(attrs.has('nameWithOptions'), 'We have the attr: nameWithOptions');
assert.false(attrs.has('nameWithTransformAndOptions'), 'We have the attr: nameWithTransformAndOptions');

let userRecord = store.push({
data: {
type: 'user',
id: '1',
attributes: {
name: 'Chris',
nameWithTransform: '@runspired',
nameWithOptions: 'Contributor',
nameWithTransformAndOptions: '@runspired contribution',
},
},
});

assert.notStrictEqual(userRecord.name, 'Chris', 'attr is correctly set: name');
assert.notStrictEqual(userRecord.nameWithTransform, '@runspired', 'attr is correctly set: nameWithTransform');
assert.notStrictEqual(userRecord.nameWithOptions, 'Contributor', 'attr is correctly set: nameWithOptions');
assert.notStrictEqual(
userRecord.nameWithTransformAndOptions,
'@runspired contribution',
'attr is correctly set: nameWithTransformAndOptions'
);
});

test('attr can be used with native syntax decorator style without parens', async function(assert) {
class User extends Model {
@attr name;
}

owner.register('model:user', User);

let UserModel = store.modelFor('user');
let attrs = UserModel.attributes;
assert.true(attrs.has('name'), 'We have the attr: name');

let userRecord = store.push({
data: {
type: 'user',
id: '1',
attributes: {
name: 'Chris',
},
},
});

assert.strictEqual(userRecord.name, 'Chris', 'attr is correctly set: name');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,18 @@ module('unit/model/rollbackAttributes - model.rollbackAttributes()', function(ho
return person;
});

assert.equal(person.get('firstName'), 'Thomas');
assert.equal(person.get('firstName'), 'Thomas', 'PreCond: we mutated firstName');
if (DEPRECATE_RECORD_LIFECYCLE_EVENT_METHODS) {
assert.equal(person.get('rolledBackCount'), 0);
assert.equal(person.get('rolledBackCount'), 0, 'PreCond: we have not yet rolled back');
}

run(() => person.rollbackAttributes());

assert.equal(person.get('firstName'), 'Tom');
assert.false(person.get('hasDirtyAttributes'));
assert.equal(person.get('firstName'), 'Tom', 'We rolled back firstName');
assert.false(person.get('hasDirtyAttributes'), 'We expect the record to be clean');

if (DEPRECATE_RECORD_LIFECYCLE_EVENT_METHODS) {
assert.equal(person.get('rolledBackCount'), 1);
assert.equal(person.get('rolledBackCount'), 1, 'We rolled back once');
}
});

Expand Down
4 changes: 2 additions & 2 deletions packages/model/addon/-private/attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ function attr(type, options) {
return computed({
get(key) {
if (DEBUG) {
if (['_internalModel', 'recordData', 'currentState'].indexOf(key) !== -1) {
if (['_internalModel', 'currentState'].indexOf(key) !== -1) {
throw new Error(
`'${key}' is a reserved property name on instances of classes extending Model. Please choose a different property name for your attr on ${this.constructor.toString()}`
);
Expand All @@ -145,7 +145,7 @@ function attr(type, options) {
},
set(key, value) {
if (DEBUG) {
if (['_internalModel', 'recordData', 'currentState'].indexOf(key) !== -1) {
if (['_internalModel', 'currentState'].indexOf(key) !== -1) {
throw new Error(
`'${key}' is a reserved property name on instances of classes extending Model. Please choose a different property name for your attr on ${this.constructor.toString()}`
);
Expand Down
6 changes: 0 additions & 6 deletions packages/model/addon/-private/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { assert, inspect, warn } from '@ember/debug';
import { computed } from '@ember/object';
import { DEBUG } from '@glimmer/env';

import { normalizeModelName } from '@ember-data/store';

import { computedMacroWithOptionalParams } from './util';

/**
Expand Down Expand Up @@ -121,10 +119,6 @@ function belongsTo(modelName, options) {
userEnteredModelName = modelName;
}

if (typeof userEnteredModelName === 'string') {
userEnteredModelName = normalizeModelName(userEnteredModelName);
}

assert(
'The first argument to belongsTo must be a string representing a model type key, not an instance of ' +
inspect(userEnteredModelName) +
Expand Down
Loading