From 1040941cf69f047d4acae3b717062b7c629e231e Mon Sep 17 00:00:00 2001 From: snewcomer Date: Sat, 19 Jan 2019 19:41:10 -0800 Subject: [PATCH 1/2] Fix nested set with recursive solution --- README.md | 17 ++++++++++++++--- addon/index.ts | 15 +++++++++++++++ addon/types/index.ts | 3 ++- tests/integration/components/changeset-test.js | 4 ++-- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 9ee2f5ad..9226192c 100644 --- a/README.md +++ b/README.md @@ -5,8 +5,6 @@ npm version Ember Observer Score -To install: - ``` ember install ember-changeset ``` @@ -17,7 +15,7 @@ ember install ember-changeset ## Updates -We have released a v2.0.0-beta. This includes a solution for deeply nested sets with one big caveat. Some history - Post v1.3.0, there was an elegant solution proposed and implemented for deeply nested sets - e.g. `changeset.set('profile.name', 'myname')`. However, this caused many issues and was reverted in v2.0.0-beta. Since `ember-changeset` relies on [Proxy](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Proxy) like behaviour, we are able to trap `changeset.set(...` and properly handle nested sets. This, however, is a problem in templates where `mut changeset.profile.name` is implicitly `set(changeset, 'profile.name')`, thus subverting our trap. This is the caveat with the v2.0.0-beta release. Although it is an improvement over v1.3.0 and should be 1-1 behaviour if you are setting at a single level - e.g. `mut changeset.name` -, nested setters don't have an ideal solution. So we are releasing v2.0.0-beta with this caveat and adding a `changeset-set` helper to use in templates. This is a work in progress. +We have released `v2.0.0-beta`. This includes a solution for deeply nested sets with one big caveat. Some history - Post v1.3.0, there was an elegant solution proposed and implemented for deeply nested sets - e.g. `changeset.set('profile.name', 'myname')`. However, this caused many issues and was reverted in v2.0.0-beta. Since `ember-changeset` relies on [Proxy](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Proxy) like behaviour, we are able to trap `changeset.set(...` and properly handle nested sets. This, however, is a problem in templates where `mut changeset.profile.name` is implicitly `set(changeset, 'profile.name')`, thus subverting our trap. This is the caveat with the v2.0.0-beta release. Although it is an improvement over v1.3.0 and should be 1-1 behaviour if you are setting at a single level - e.g. `mut changeset.name` -, nested setters don't have an ideal solution. So we are releasing v2.0.0-beta with this caveat and adding a `changeset-set` template helper. This is a work in progress. ## Philosophy @@ -124,6 +122,19 @@ In the above example, when the input changes, only the changeset's internal valu On rollback, all changes are dropped and the underlying Object is left untouched. +## Changeset template helper +`ember-changeset` overrides `set` in order to handle deeply nested setters. `mut` is simply an alias for `set(changeset`, thus we provide a `changeset-set` template helper. + +```hbs +
+ +
+``` + ## Disabling Automatic Validation The default behavior of `Changeset` is to automatically validate a field when it is set. Automatic validation can be disabled by passing `skipValidate` as an option when creating a changeset. diff --git a/addon/index.ts b/addon/index.ts index be3d027e..432eed2f 100644 --- a/addon/index.ts +++ b/addon/index.ts @@ -739,10 +739,25 @@ export function changeset( if (key.indexOf('.') > -1) { // Adds new CHANGE and avoids ember intenals setting directly on model // TODO: overriding `set(changeset, )` doesnt work + this._setTopLevel(key, value); return this.setUnknownProperty(key, value); } else { return this._super(...arguments); } + }, + + _setTopLevel ( + key: string, + value: T + ): T { + let [topLevel, rest] = key.split('.'); + set(this.get(topLevel), rest, value); + + if (rest.indexOf('.') > -1) { + return this._setTopLevel(rest, value); + } + + return value; } } diff --git a/addon/types/index.ts b/addon/types/index.ts index c7226db5..4e02d0c5 100644 --- a/addon/types/index.ts +++ b/addon/types/index.ts @@ -119,5 +119,6 @@ export interface ChangesetDef extends Any { _valueFor: (s: string) => any, _notifyVirtualProperties: (keys?: string[]) => void, _rollbackKeys: () => Array, - _deleteKey: (objName: InternalMapKey, key: string) => void + _deleteKey: (objName: InternalMapKey, key: string) => void, + _setNestedKey: (key: string, value: T) => T }; diff --git a/tests/integration/components/changeset-test.js b/tests/integration/components/changeset-test.js index 2f56a45d..69901309 100644 --- a/tests/integration/components/changeset-test.js +++ b/tests/integration/components/changeset-test.js @@ -161,7 +161,7 @@ module('Integration | Helper | changeset', function(hooks) { // assert.equal(find('h1').textContent.trim(), 'foo bar', 'should update observable value'); }); - skip('nested object updates when set without a validator', async function(assert) { + test('nested object updates when set without a validator', async function(assert) { let data = { person: { firstName: 'Jim', lastName: 'Bob' } }; let changeset = new Changeset(data); this.set('changeset', changeset); @@ -216,7 +216,7 @@ module('Integration | Helper | changeset', function(hooks) { } }); - skip('nested object updates when set with async validator', async function(assert) { + test('nested object updates when set with async validator', async function(assert) { let data = { person: { firstName: 'Jim' } }; let validator = () => resolve(true); let c = new Changeset(data, validator); From fa08d25762fe51cd6eb2c4642fabdcaa5c9876f3 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Sat, 19 Jan 2019 19:50:15 -0800 Subject: [PATCH 2/2] revert prod changes...terrible idea --- addon/index.ts | 15 --------------- addon/types/index.ts | 3 +-- tests/integration/components/changeset-test.js | 4 ++-- 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/addon/index.ts b/addon/index.ts index 432eed2f..be3d027e 100644 --- a/addon/index.ts +++ b/addon/index.ts @@ -739,25 +739,10 @@ export function changeset( if (key.indexOf('.') > -1) { // Adds new CHANGE and avoids ember intenals setting directly on model // TODO: overriding `set(changeset, )` doesnt work - this._setTopLevel(key, value); return this.setUnknownProperty(key, value); } else { return this._super(...arguments); } - }, - - _setTopLevel ( - key: string, - value: T - ): T { - let [topLevel, rest] = key.split('.'); - set(this.get(topLevel), rest, value); - - if (rest.indexOf('.') > -1) { - return this._setTopLevel(rest, value); - } - - return value; } } diff --git a/addon/types/index.ts b/addon/types/index.ts index 4e02d0c5..c7226db5 100644 --- a/addon/types/index.ts +++ b/addon/types/index.ts @@ -119,6 +119,5 @@ export interface ChangesetDef extends Any { _valueFor: (s: string) => any, _notifyVirtualProperties: (keys?: string[]) => void, _rollbackKeys: () => Array, - _deleteKey: (objName: InternalMapKey, key: string) => void, - _setNestedKey: (key: string, value: T) => T + _deleteKey: (objName: InternalMapKey, key: string) => void }; diff --git a/tests/integration/components/changeset-test.js b/tests/integration/components/changeset-test.js index 69901309..2f56a45d 100644 --- a/tests/integration/components/changeset-test.js +++ b/tests/integration/components/changeset-test.js @@ -161,7 +161,7 @@ module('Integration | Helper | changeset', function(hooks) { // assert.equal(find('h1').textContent.trim(), 'foo bar', 'should update observable value'); }); - test('nested object updates when set without a validator', async function(assert) { + skip('nested object updates when set without a validator', async function(assert) { let data = { person: { firstName: 'Jim', lastName: 'Bob' } }; let changeset = new Changeset(data); this.set('changeset', changeset); @@ -216,7 +216,7 @@ module('Integration | Helper | changeset', function(hooks) { } }); - test('nested object updates when set with async validator', async function(assert) { + skip('nested object updates when set with async validator', async function(assert) { let data = { person: { firstName: 'Jim' } }; let validator = () => resolve(true); let c = new Changeset(data, validator);