From 79a08b0f5fd49c3eb61b26e9baee52aa05f0402c Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 6 Mar 2019 11:43:01 -0800 Subject: [PATCH 1/7] initial spike --- .../0000-ember-data-record-data-operations.md | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 text/0000-ember-data-record-data-operations.md diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md new file mode 100644 index 0000000000..9b29b87181 --- /dev/null +++ b/text/0000-ember-data-record-data-operations.md @@ -0,0 +1,132 @@ +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- RFC PR: (leave this empty) +- Ember Issue: (leave this empty) + +# Record Data Operations + +## Summary + +Introduces `Operations` for local mutations of data which are small objects describing a +mutation + +## Motivation + +- better organize API surface +- provide clear communication channels between primitives +- reduce API surface by disentangling DS.belongsTo / DS.hasMany from RecordData +- foundation for future operations based APIs (relationships, canonical state patches, + logging, serialization, debugging APIs, reverse operations, transactions) +- improved factoring of internals +- easy deprecation path to singleton RD support + +## Detailed design + +- synchronous, because `setters` need to complete for "sync get" +- single operations, not batches, as "transactions" or "transforms" (orbit) would be a + higher level primitive introduced lated if desired at a different position within the + architecture +- op-codes are stable for serializability +- operations don't have identifiers, "transactions" or "transforms" would +- operations that fail should throw +- it is valid for a RecordData implementation to ignore an operation + (but we should probably encourage logging ignored operations) +- operations are only for mutations of local state (at this time), use `push` for canonical + _note: it is likely that we want operations within `push` for describing partial updates + and a `relationships` RFC in particular will likely desire this and discuss it if so_ + +```typescript +export interface Operation { + // the operation to perform + op: string; + // the record to be operated on + record: RecordIdentifier; + // the name of the property on the record to be mutated (if any) + property?: string; + // the new value for the property on the record (if any) + // if this key is present, this value can be anything but `undefined` + value?: Identifier | null | array | string | object | number | boolean; +} + +export default interface RecordData { + performMutation(operation: Operation): void +} +``` + +Example operations + +```ts +interface ReplaceAttributeOperation extends Operation { + op: 'replaceAttribute'; + record: RecordIdentifier; + property: string; // "attr" propertyName + value: any; +} +interface AddToRelatedRecordsOperation extends Operation { + op: 'addToRelatedRecords'; + record: RecordIdentifier; + property: string; // "relationship" propertyName + value: RecordIdentifier; // related record +} +interface RemoveFromRelatedRecordsOperation extends Operation { + op: 'removeFromRelatedRecords'; + record: RecordIdentifier; + property: string; // "relationship" propertyName + value: RecordIdentifier; // related record +} +``` + +### Existing APIs this would supercede + +```ts +interface RecordData { + setDirtyAttribute(key, value) + addToHasMany(key, recordDatas, index) + removeFromHasMany(key, recordDatas) + setHasMany(key, recordDatas) + setBelongsTo(key, recordData) + removeFromInverseRelationships(isNew) + rollbackAttributes() + clientDidCreate() + didDelete() +} +``` + +Additionally we may consider modeling these APIs as operations as well + +```ts + didCommit(data) // see also willCommit + _initRecordCreateOptions(options) // paired with clientDidCreate + reset() +``` + +## How we teach this + +> What names and terminology work best for these concepts and why? How is this +idea best presented? As a continuation of existing Ember patterns, or as a +wholly new one? + +> Would the acceptance of this proposal mean the Ember guides must be +re-organized or altered? Does it change how Ember is taught to new users +at any level? + +> How should this feature be introduced and taught to existing Ember +users? + +## Drawbacks + +> Why should we *not* do this? Please consider the impact on teaching Ember, +on the integration of this feature with other existing and planned features, +on the impact of the API churn on existing apps, etc. + +> There are tradeoffs to choosing any path, please attempt to identify them here. + +## Alternatives + +> What other designs have been considered? What is the impact of not doing this? + +> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. + +## Unresolved questions + +> Optional, but suggested for first drafts. What parts of the design are still +TBD? From 87d5dd598ace275f43f270863bb56bba3cf57db8 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 6 Mar 2019 13:17:33 -0800 Subject: [PATCH 2/7] update summary --- text/0000-ember-data-record-data-operations.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md index 9b29b87181..b688c5a9e0 100644 --- a/text/0000-ember-data-record-data-operations.md +++ b/text/0000-ember-data-record-data-operations.md @@ -6,8 +6,8 @@ ## Summary -Introduces `Operations` for local mutations of data which are small objects describing a -mutation +Introduces `RecordData.performMutation` which takes an `Operation` for local mutations of +data. `Operations` are small, serializable objects describing a mutation. ## Motivation @@ -87,16 +87,16 @@ interface RecordData { removeFromInverseRelationships(isNew) rollbackAttributes() clientDidCreate() - didDelete() } ``` Additionally we may consider modeling these APIs as operations as well ```ts - didCommit(data) // see also willCommit - _initRecordCreateOptions(options) // paired with clientDidCreate - reset() + didDelete() // cannonical update + didCommit(data) // cannonical update, see also willCommit + _initRecordCreateOptions(options) // 2nd phase of create, paired with clientDidCreate + reset() // similar to rollbackAttributes ``` ## How we teach this From 8127cd755866783047e6acf33f340472a3dcfc74 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Wed, 6 Mar 2019 13:34:39 -0800 Subject: [PATCH 3/7] additional motivations --- .../0000-ember-data-record-data-operations.md | 47 ++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md index b688c5a9e0..bed87a71d9 100644 --- a/text/0000-ember-data-record-data-operations.md +++ b/text/0000-ember-data-record-data-operations.md @@ -6,8 +6,8 @@ ## Summary -Introduces `RecordData.performMutation` which takes an `Operation` for local mutations of -data. `Operations` are small, serializable objects describing a mutation. +Introduces `RecordData.performMutation` which takes an `Operation` for local mutations + of data. `Operations` are small, serializable objects describing a mutation. ## Motivation @@ -19,6 +19,49 @@ data. `Operations` are small, serializable objects describing a mutation. - improved factoring of internals - easy deprecation path to singleton RD support +`Operations` are a foundational concept for `acid` transactions. Without the ability to + describe an `operation` and a clear mental model of what operations exist and achieve, + it is difficult to understand how an action affects state. Granularity and clarity is + **key**. + +In more layman terms, let's take a look at a few `RecordData` methods and how they might +better be understood as operations, using the originally proposed singleton `RecordData` +APIs for mutating an attribute or a relationship. + +```ts +interface RecordData { + setAttr(modelName: string, id?: string, clientId?: string, key: string, value: string) {} + setBelongsTo(modelName: string, id?: string, clientId?: string, key: string, jsonApiResource) {} + addToHasMany(modelName: string, id?: string, clientId?: string, key: string, jsonApiResources, idx: number) {} + removeFromHasMany(modelName: string, id?: string, clientId?: string, key: string, jsonApiResources) {} +} +``` + +The above is just one snapshot of the roughly dozen APIs relating to mutation of the ~25 +APIs present on `RecordData`. + +The "soup" of available methods prevents someone from knowing at a glance which methods +are for mutations, and whether these methods are for mutating local state, or communicating +remote state changes. + +It also means that developers must memorize a large API surface area, and don't have a clear +picture of how these methods relate to each other in terms of what they achieve. + +### Eliminating unnecessary surface area and better decoupling + +Implementations proving a path towards a nicer relationship graph have shown that the distinction + between `belongsTo` and `hasMany` at the `RecordData` level is unnecessary, and only leads to + extraneous method branching throughout the rest of `ember-data`'s primtives. + +Furthermore, some `Record` and `RecordData` implementations may not provide relationships at all, + leaving unnecessary dangling methods around to confuse folks exploring the codebase while debugging. + +With these improvements to `RecordData` and associated changes to relationships and `Record` classes, +`ember-data` will no longer need to be coupled to the existing understanding or semantics of `belongsTo` +and `hasMany`, allowing custom records to define new types of relationships more easily (such as `map`, +`set`, `union`, `enum`) without shoehorning them through `attribute` methods or fudging them into `belongsTo` +or `hasMany` in form. + ## Detailed design - synchronous, because `setters` need to complete for "sync get" From a8940514ebb3fd92a84b81f64951681c36211bac Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 8 Mar 2019 19:19:36 -0800 Subject: [PATCH 4/7] Update text/0000-ember-data-record-data-operations.md Co-Authored-By: runspired --- text/0000-ember-data-record-data-operations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md index bed87a71d9..b920ca7e9a 100644 --- a/text/0000-ember-data-record-data-operations.md +++ b/text/0000-ember-data-record-data-operations.md @@ -19,7 +19,7 @@ Introduces `RecordData.performMutation` which takes an `Operation` for local mut - improved factoring of internals - easy deprecation path to singleton RD support -`Operations` are a foundational concept for `acid` transactions. Without the ability to +`Operations` are a foundational concept for [ACID](https://en.wikipedia.org/wiki/ACID_(computer_science)) transactions. Without the ability to describe an `operation` and a clear mental model of what operations exist and achieve, it is difficult to understand how an action affects state. Granularity and clarity is **key**. From ffa692686503537ff5a6ed915eb56d7df557fef5 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Mon, 11 Mar 2019 10:34:14 -0700 Subject: [PATCH 5/7] add link to original RFC --- text/0000-ember-data-record-data-operations.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md index b920ca7e9a..f62ee0b8ba 100644 --- a/text/0000-ember-data-record-data-operations.md +++ b/text/0000-ember-data-record-data-operations.md @@ -25,8 +25,9 @@ Introduces `RecordData.performMutation` which takes an `Operation` for local mut **key**. In more layman terms, let's take a look at a few `RecordData` methods and how they might -better be understood as operations, using the originally proposed singleton `RecordData` -APIs for mutating an attribute or a relationship. +better be understood as operations, using the originally proposed singleton +[`RecordData`](https://github.com/emberjs/rfcs/pull/293) APIs for mutating an attribute +or a relationship. ```ts interface RecordData { From f987d5d4bb38c00154ea7fcf8b492d05a034bde7 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Mon, 11 Mar 2019 10:35:18 -0700 Subject: [PATCH 6/7] remove use of singleton language --- text/0000-ember-data-record-data-operations.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md index f62ee0b8ba..1082b9f198 100644 --- a/text/0000-ember-data-record-data-operations.md +++ b/text/0000-ember-data-record-data-operations.md @@ -25,9 +25,8 @@ Introduces `RecordData.performMutation` which takes an `Operation` for local mut **key**. In more layman terms, let's take a look at a few `RecordData` methods and how they might -better be understood as operations, using the originally proposed singleton -[`RecordData`](https://github.com/emberjs/rfcs/pull/293) APIs for mutating an attribute -or a relationship. +better be understood as operations, using the originally proposed [`RecordData`](https://github.com/emberjs/rfcs/pull/293) +APIs for mutating an attribute or a relationship. ```ts interface RecordData { From af6e0bf859ccae9c943a0bb3aa30b477dd777be6 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Mon, 11 Mar 2019 12:04:21 -0700 Subject: [PATCH 7/7] provide list of ops --- .../0000-ember-data-record-data-operations.md | 140 ++++++++++++++---- 1 file changed, 108 insertions(+), 32 deletions(-) diff --git a/text/0000-ember-data-record-data-operations.md b/text/0000-ember-data-record-data-operations.md index 1082b9f198..46870efa9c 100644 --- a/text/0000-ember-data-record-data-operations.md +++ b/text/0000-ember-data-record-data-operations.md @@ -24,28 +24,41 @@ Introduces `RecordData.performMutation` which takes an `Operation` for local mut it is difficult to understand how an action affects state. Granularity and clarity is **key**. -In more layman terms, let's take a look at a few `RecordData` methods and how they might -better be understood as operations, using the originally proposed [`RecordData`](https://github.com/emberjs/rfcs/pull/293) -APIs for mutating an attribute or a relationship. +Let's take a look at a the [`RecordData`](https://github.com/emberjs/rfcs/pull/293) +methods that might better be understood as operations using [`identifiers`](https://github.com/emberjs/rfcs/pull/403). + +**Methods That Mutate Record Properties** ```ts interface RecordData { - setAttr(modelName: string, id?: string, clientId?: string, key: string, value: string) {} - setBelongsTo(modelName: string, id?: string, clientId?: string, key: string, jsonApiResource) {} - addToHasMany(modelName: string, id?: string, clientId?: string, key: string, jsonApiResources, idx: number) {} - removeFromHasMany(modelName: string, id?: string, clientId?: string, key: string, jsonApiResources) {} + addToHasMany(key: string, recordDatas, idx: number) {} + removeFromHasMany(key: string, recordDatas) {} + removeFromInverseRelationships(isNew: boolean) {} + setBelongsTo(key: string, recordData) {} + setDirtyAttribute(key: string, value: any) {} + setHasMany(key: string, recordDatas) {} + rollbackAttributes() {} } ``` -The above is just one snapshot of the roughly dozen APIs relating to mutation of the ~25 -APIs present on `RecordData`. +**Methods That Mutate Record State** + +```ts +interface RecordData { + clientDidCreate() // "addRecord" + // in the original RFC but we failed to implement (but is needed) + clientDidDelete() // "removeRecord" + unloadRecord() // "unloadRecord" +} +``` The "soup" of available methods prevents someone from knowing at a glance which methods are for mutations, and whether these methods are for mutating local state, or communicating remote state changes. It also means that developers must memorize a large API surface area, and don't have a clear -picture of how these methods relate to each other in terms of what they achieve. +picture of how these methods relate to each other in terms of what they achieve. Organizing +mutations as `operations` will help us provide both clearer mental model and a better decoupling. ### Eliminating unnecessary surface area and better decoupling @@ -95,21 +108,24 @@ export default interface RecordData { } ``` -Example operations +### Operations + +**Operations That Mutate Record Properties** + +**`addToHasMany`** ```ts -interface ReplaceAttributeOperation extends Operation { - op: 'replaceAttribute'; - record: RecordIdentifier; - property: string; // "attr" propertyName - value: any; -} interface AddToRelatedRecordsOperation extends Operation { op: 'addToRelatedRecords'; record: RecordIdentifier; property: string; // "relationship" propertyName value: RecordIdentifier; // related record } +``` + +**`removeFromHasMany`** + +```ts interface RemoveFromRelatedRecordsOperation extends Operation { op: 'removeFromRelatedRecords'; record: RecordIdentifier; @@ -118,30 +134,90 @@ interface RemoveFromRelatedRecordsOperation extends Operation { } ``` -### Existing APIs this would supercede +**`setBelongsTo`** ```ts -interface RecordData { - setDirtyAttribute(key, value) - addToHasMany(key, recordDatas, index) - removeFromHasMany(key, recordDatas) - setHasMany(key, recordDatas) - setBelongsTo(key, recordData) - removeFromInverseRelationships(isNew) - rollbackAttributes() - clientDidCreate() +interface ReplaceRelatedRecordOperation extends Operation { + op: 'replaceRelatedRecord'; + record: RecordIdentity; + property: string; + value: RecordIdentity; } ``` -Additionally we may consider modeling these APIs as operations as well +**`setDirtyAttribute`** ```ts - didDelete() // cannonical update - didCommit(data) // cannonical update, see also willCommit - _initRecordCreateOptions(options) // 2nd phase of create, paired with clientDidCreate - reset() // similar to rollbackAttributes +interface ReplaceAttributeOperation extends Operation { + op: 'replaceAttribute'; + record: RecordIdentifier; + property: string; // "attr" propertyName + value: any; +} ``` +**`setHasMany`** + +```ts +interface ReplaceRelatedRecordsOperation extends Operation { + op: 'replaceRelatedRecords'; + record: RecordIdentity; + property: string; + value: RecordIdentity[]; +} +``` + +**`rollbackAttributes`** + +Once "logs" are present this could be modeled as a series of reverse operations. + +```ts +interface RollbackAttributesOperation extends Operation { + op: 'rollbackAttributes', + record: RecordIdentity; +} +``` + +**Operations That Mutate Record State** + +**`clientDidCreate`** + +```ts +interface AddRecordOperation extends Operation { + op: 'addRecord'; + record: Record; +} +``` + +**`clientDidDelete`** + +```ts +interface RemoveRecordOperation extends Operation { + op: 'removeRecord'; + record: Record; +} +``` + +**`unloadRecord`** + +```ts +interface UnloadRecordOperation extends Operation { + op: 'unloadRecord'; + record: Record; +} +``` + +### _initRecordCreateOptions + +This API is problematic and hard to model, we need something, let's discuss. + +### removeFromInverseRelationships + +This API should be eliminated entirely, removeRecord and unloadRecord + would take care of the use case *if* recordData tracks whether it is + `isNew` and `isDeleted`. + + ## How we teach this > What names and terminology work best for these concepts and why? How is this