Skip to content

Commit

Permalink
fix some tests and add deprecation infra for preserving legacy reset …
Browse files Browse the repository at this point in the history
…behavior
  • Loading branch information
runspired committed Aug 30, 2023
1 parent 9a39d6a commit c17ffa9
Show file tree
Hide file tree
Showing 12 changed files with 301 additions and 103 deletions.
1 change: 1 addition & 0 deletions ember-data-types/q/record-data-schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface RelationshipSchema {
async: boolean; // controls inverse unloading "client side delete semantics" so we should replace that with a real flag
polymorphic?: boolean;
inverse: string | null; // property key on the related type (if any)
resetOnRemoteUpdate?: false; // manages the deprecation `DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE`
[key: string]: unknown;
};
// inverse?: string | null;
Expand Down
7 changes: 7 additions & 0 deletions packages/graph/src/-private/-edge-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export interface UpgradedMeta {
isImplicit: boolean;
isCollection: boolean;
isPolymorphic: boolean;
resetOnRemoteUpdate: boolean;

inverseKind: 'hasMany' | 'belongsTo' | 'implicit';
/**
Expand Down Expand Up @@ -163,6 +164,10 @@ function syncMeta(definition: UpgradedMeta, inverseDefinition: UpgradedMeta) {
definition.inverseIsCollection = inverseDefinition.isCollection;
definition.inverseIsPolymorphic = inverseDefinition.isPolymorphic;
definition.inverseIsImplicit = inverseDefinition.isImplicit;
const resetOnRemoteUpdate =
definition.resetOnRemoteUpdate === false || inverseDefinition.resetOnRemoteUpdate === false ? false : true;
definition.resetOnRemoteUpdate = resetOnRemoteUpdate;
inverseDefinition.resetOnRemoteUpdate = resetOnRemoteUpdate;
}

function upgradeMeta(meta: RelationshipSchema): UpgradedMeta {
Expand All @@ -183,6 +188,8 @@ function upgradeMeta(meta: RelationshipSchema): UpgradedMeta {
niceMeta.inverseIsImplicit = (options && options.inverse === null) || BOOL_LATER;
niceMeta.inverseIsCollection = BOOL_LATER;

niceMeta.resetOnRemoteUpdate = options && options.resetOnRemoteUpdate === false ? false : true;

return niceMeta;
}

Expand Down
69 changes: 67 additions & 2 deletions packages/graph/src/-private/operations/replace-related-records.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { assert } from '@ember/debug';
import { assert, deprecate } from '@ember/debug';

import { DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE } from '@ember-data/deprecations';
import { DEBUG } from '@ember-data/env';
import type { StableRecordIdentifier } from '@ember-data/types/q/identifier';

import { _addLocal, _removeLocal, diffCollection } from '../-diff';
import type { ReplaceRelatedRecordsOperation } from '../-operations';
import { isBelongsTo, isHasMany, notifyChange } from '../-utils';
import { isBelongsTo, isHasMany, isNew, notifyChange } from '../-utils';
import { assertPolymorphicType } from '../debug/assert-polymorphic-type';
import type { CollectionEdge } from '../edges/collection';
import type { Graph } from '../graph';
Expand Down Expand Up @@ -182,6 +183,70 @@ function replaceRelatedRecordsRemote(graph: Graph, op: ReplaceRelatedRecordsOper
// the associated ManyArray
relationship._diff = diff;

debugger;
if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
// only do this for legacy hasMany, not collection
// and provide a way to incrementally migrate
if (relationship.definition.kind === 'hasMany' && relationship.definition.resetOnRemoteUpdate !== false) {
const deprecationInfo: {
removals: StableRecordIdentifier[];
additions: StableRecordIdentifier[];
triggered: boolean;
} = {
removals: [],
additions: [],
triggered: false,
};
if (relationship.removals) {
deprecationInfo.triggered = true;
relationship.isDirty = true;
relationship.removals.forEach((identifier) => {
deprecationInfo.removals.push(identifier);
// reverse the removal
// if we are still in removals at this point then
// we were not "committed" which means we are present
// in the remoteMembers. So we "add back" on the inverse.
addToInverse(graph, identifier, definition.inverseKey, op.record, isRemote);
});
relationship.removals = null;
}
if (relationship.additions) {
relationship.additions.forEach((identifier) => {
// reverse the addition
// if we are still in additions at this point then
// we were not "committed" which means we are not present
// in the remoteMembers. So we "remove" from the inverse.
// however we only do this if we are not a "new" record.
if (!isNew(identifier)) {
deprecationInfo.triggered = true;
deprecationInfo.additions.push(identifier);
relationship.isDirty = true;
relationship.additions!.delete(identifier);
removeFromInverse(graph, identifier, definition.inverseKey, op.record, isRemote);
}
});
if (relationship.additions.size === 0) {
relationship.additions = null;
}
}

if (deprecationInfo.triggered) {
deprecate(
`EmberData is changing the default semantics of updates to the remote state of relationships.\n\nThe following local state was cleared but will not be once this deprecation is resolved:\n\n\tAdded: [${deprecationInfo.additions
.map((i) => i.lid)
.join(', ')}]\n\tRemoved: [${deprecationInfo.removals.map((i) => i.lid).join(', ')}]`,
false,
{
id: 'ember-data:deprecate-relationship-remote-update-clearing-local-state',
for: 'ember-data',
since: { enabled: '5.3', available: '5.3' },
until: '6.0',
}
);
}
}
}

if (diff.changed) {
flushCanonical(graph, relationship);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export const DEPRECATE_NON_STRICT_TYPES: boolean;
export const DEPRECATE_NON_STRICT_ID: boolean;
export const DEPRECATE_LEGACY_IMPORTS: boolean;
export const DEPRECATE_NON_UNIQUE_PAYLOADS: boolean;
export const DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE: boolean;
122 changes: 111 additions & 11 deletions packages/private-build-infra/virtual-packages/deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ export const DEPRECATE_3_12 = '3.12';
* e.g. `app/models/foo/bar-bem.js` must have a type of `foo/bar-bem`
*
* @property DEPRECATE_NON_STRICT_TYPES
* @since 5.2
* @since 5.3
* @until 6.0
* @public
*/
export const DEPRECATE_NON_STRICT_TYPES = '5.2';
export const DEPRECATE_NON_STRICT_TYPES = '5.3';

/**
* **id: ember-data:deprecate-non-strict-id**
Expand All @@ -122,11 +122,11 @@ export const DEPRECATE_NON_STRICT_TYPES = '5.2';
* custom identifier configuration should provide a string ID.
*
* @property DEPRECATE_NON_STRICT_ID
* @since 5.2
* @since 5.3
* @until 6.0
* @public
*/
export const DEPRECATE_NON_STRICT_ID = '5.2';
export const DEPRECATE_NON_STRICT_ID = '5.3';

/**
* **id: <none yet assigned>**
Expand All @@ -135,8 +135,8 @@ export const DEPRECATE_NON_STRICT_ID = '5.2';
* chains are used to watch for changes on any EmberData RecordArray, ManyArray
* or PromiseManyArray.
*
* Support for these chains is currently guarded by the inactive deprecation flag
* listed here.
* Support for these chains is currently guarded by the deprecation flag
* listed here, enabling removal of the behavior if desired.
*
* @property DEPRECATE_COMPUTED_CHAINS
* @since 5.0
Expand All @@ -158,21 +158,121 @@ export const DEPRECATE_COMPUTED_CHAINS = '5.0';
* of defaults.
*
* @property DEPRECATE_LEGACY_IMPORTS
* @since 5.2
* @since 5.3
* @until 6.0
* @public
*/
export const DEPRECATE_LEGACY_IMPORTS = '5.2';
export const DEPRECATE_LEGACY_IMPORTS = '5.3';

/* PLANNED
/**
* **id: ember-data:deprecate-non-unique-collection-payloads**
*
* Deprecates when the data for a hasMany relationship contains
* duplicate identifiers.
*
* @property DEPRECATE_NON_UNIQUE_PAYLOADS
* @since 5.2
* @since 5.3
* @until 6.0
* @public
*/
export const DEPRECATE_NON_UNIQUE_PAYLOADS = '5.3';

/**
* **id: ember-data:deprecate-relationship-remote-update-clearing-local-state**
*
* Deprecates when a relationship is updated remotely and the local state
* is cleared of all changes except for "new" records.
*
* Instead, any records not present in the new payload will be considered
* "removed" while any records present in the new payload will be considered "added".
*
* This allows us to "commit" local additions and removals, preserving any additions
* or removals that are not yet reflected in the remote state.
*
* For instance, given the following initial state:
*
* remote: A, B, C
* local: add D, E
* remove B, C
* => A, D, E
*
*
* If after an update, the remote state is now A, B, D, F then the new state will be
*
* remote: A, B, D, F
* local: add E
* remove B
* => A, D, E, F
*
* Under the old behavior the updated local state would instead have been
* => A, B, D, F
*
* Similarly, if a belongsTo remote State was A while its local state was B,
* then under the old behavior if the remote state changed to C, the local state
* would be updated to C. Under the new behavior, the local state would remain B.
*
* If the remote state was A while its local state was `null`, then under the old
* behavior if the remote state changed to C, the local state would be updated to C.
* Under the new behavior, the local state would remain `null`.
*
* Thus the new correct mental model is that the state of the relationship at any point
* in time is whatever the most recent remote state is, plus any local additions or removals
* you have made that have not yet been reflected by the remote state.
*
* > Note: The old behavior extended to modifying the inverse of a relationship. So if
* > you had local state not reflected in the new remote state, inverses would be notified
* > and their state reverted as well when "resetting" the relationship.
* > Under the new behavior, since the local state is preserved the inverses will also
* > not be reverted.
*
* ### Resolving this deprecation
*
* Resolving this deprecation can be done individually for each relationship
* or globally for all relationships.
*
* To resolve it globally, set the `DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE`
* to `false` in ember-cli-build.js
*
* ```js
* let app = new EmberApp(defaults, {
* emberData: {
* deprecations: {
* // set to false to strip the deprecated code (thereby opting into the new behavior)
* DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE: false
* }
* }
* })
* ```
*
* To resolve this deprecation on an individual relationship, adjust the `options` passed to
* the relationship. For relationships with inverses, both sides MUST be migrated to the new
* behavior at the same time.
*
* ```js
* class Person extends Model {
* @hasMany('person', {
* async: false,
* inverse: null,
* resetOnRemoteUpdate: false
* }) children;
*
* @belongsTo('person', {
* async: false,
* inverse: null,
* resetOnRemoteUpdate: false
* }) parent;
* }
* ```
*
* > Note: false is the only valid value here, all other values (including missing)
* > will be treated as true, where `true` is the legacy behavior that is now deprecated.
*
* Once you have migrated all relationships, you can remove the the resetOnRemoteUpdate
* option and set the deprecation flag to false in ember-cli-build.
*
* @property DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE
* @since 5.3
* @until 6.0
* @public
*/
export const DEPRECATE_NON_UNIQUE_PAYLOADS = '5.2';
export const DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE = '5.3';
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ function gte(EDVersion: string, DeprecationVersion: string): boolean {
export function deprecatedTest(
testName: string,
deprecation: {
until: `${number}.${number}.${number}`;
until: `${number}.${number}`;
id: string;
count: number;
// this test should only run in debug mode
debugOnly?: boolean;
// this test should remain in the codebase but
// should be refactored to no longer use the deprecated feature
refactor?: boolean;
},
testCallback: <T extends TestContext>(this: T, assert: Assert) => void | Promise<void>
Expand Down
Loading

0 comments on commit c17ffa9

Please sign in to comment.