Skip to content

Commit

Permalink
chore: log a warning when DataStore is ignoring an attempted PK update (
Browse files Browse the repository at this point in the history
  • Loading branch information
svidgen committed Dec 9, 2022
1 parent 4725b5f commit cde60fc
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
24 changes: 24 additions & 0 deletions packages/datastore/__tests__/DataStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,8 @@ describe('DataStore tests', () => {
});

test('Id cannot be changed inside copyOf', () => {
const consoleWarn = jest.spyOn(console, 'warn');

const { Model } = initSchema(testSchema()) as {
Model: PersistentModelConstructor<Model>;
};
Expand All @@ -2111,6 +2113,16 @@ describe('DataStore tests', () => {

// ID should be kept the same
expect(model1.id).toBe(model2.id);

// we should always be told *in some way* when an "update" will not actually
// be applied. for now, this is a warning, because throwing an error, updating
// the record's PK, or creating a new record are all breaking changes.
expect(consoleWarn).toHaveBeenCalledWith(
expect.stringContaining(
"copyOf() does not update PK fields. The 'id' update is being ignored."
),
expect.objectContaining({ source: model1 })
);
});

test('Optional field can be initialized with undefined', () => {
Expand Down Expand Up @@ -3629,6 +3641,8 @@ describe('DataStore tests', () => {
});

test('postId cannot be changed inside copyOf', () => {
const consoleWarn = jest.spyOn(console, 'warn');

const { PostCustomPK } = initSchema(testSchema()) as {
PostCustomPK: PersistentModelConstructor<PostCustomPKType>;
};
Expand All @@ -3645,6 +3659,16 @@ describe('DataStore tests', () => {

// postId should be kept the same
expect(model1.postId).toBe(model2.postId);

// we should always be told *in some way* when an "update" will not actually
// be applied. for now, this is a warning, because throwing an error, updating
// the record's PK, or creating a new record are all breaking changes.
expect(consoleWarn).toHaveBeenCalledWith(
expect.stringContaining(
"copyOf() does not update PK fields. The 'postId' update is being ignored."
),
expect.objectContaining({ source: model1 })
);
});

test('Optional field can be initialized with undefined', () => {
Expand Down
10 changes: 9 additions & 1 deletion packages/datastore/src/datastore/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,15 @@ const createModelClass = <T extends PersistentModel>(

const keyNames = extractPrimaryKeyFieldNames(modelDefinition);
// Keys are immutable
keyNames.forEach(key => ((draft as Object)[key] = source[key]));
keyNames.forEach(key => {
if (draft[key] !== source[key]) {
logger.warn(
`copyOf() does not update PK fields. The '${key}' update is being ignored.`,
{ source }
);
}
(draft as Object)[key] = source[key];
});

const modelValidator = validateModelFields(modelDefinition);
Object.entries(draft).forEach(([k, v]) => {
Expand Down

0 comments on commit cde60fc

Please sign in to comment.