From e8ec97c820565d4bd3ad510a238e05c13bff7293 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Tue, 10 Sep 2019 00:12:27 +0100 Subject: [PATCH 1/2] Skip writing undefined fields and warn in dev --- src/operations/write.ts | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/src/operations/write.ts b/src/operations/write.ts index 1f76a08..d2a48ff 100644 --- a/src/operations/write.ts +++ b/src/operations/write.ts @@ -43,6 +43,7 @@ interface Context { store: Store; variables: Variables; fragments: Fragments; + isOptimistic?: boolean; schemaPredicates?: SchemaPredicates; } @@ -53,11 +54,8 @@ export const write = ( data: Data ): WriteResult => { initStoreState(0); - const result = startWrite(store, request, data); - clearStoreState(); - return result; }; @@ -107,6 +105,10 @@ export const writeOptimistic = ( schemaPredicates: store.schemaPredicates, }; + if (process.env.NODE_ENV === 'development') { + ctx.isOptimistic = true; + } + const operationName = ctx.store.getRootKey(operation.operation); if (operationName === ctx.store.getRootKey('mutation')) { const select = getSelectionSet(operation); @@ -204,12 +206,31 @@ const writeSelection = ( if (isQuery) addDependency(fieldKey); - if ( - process.env.NODE_ENV !== 'production' && - ctx.schemaPredicates && - typename - ) { - ctx.schemaPredicates.isFieldAvailableOnType(typename, fieldName); + if (process.env.NODE_ENV !== 'production') { + if (fieldValue === undefined) { + const advice = ctx.isOptimistic + ? '\nYour optimistic result may be missing a field!' + : ''; + + const expected = + node.selectionSet === undefined + ? 'scalar (number, boolean, etc)' + : 'selection set'; + + warning( + false, + 'Invalid value: The field at `' + + fieldKey + + '` is `undefined`, but the GraphQL query expects a ' + + expected + + ' for this field.' + + advice + ); + + continue; // Skip this field + } else if (ctx.schemaPredicates && typename) { + ctx.schemaPredicates.isFieldAvailableOnType(typename, fieldName); + } } if (node.selectionSet === undefined) { From a8fb2871b8f0b06bfb449502f2807483aeb97e9f Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Tue, 10 Sep 2019 00:28:05 +0100 Subject: [PATCH 2/2] Add test for not writing undefined --- src/operations/write.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/operations/write.test.ts b/src/operations/write.test.ts index 117218d..4a43f4c 100644 --- a/src/operations/write.test.ts +++ b/src/operations/write.test.ts @@ -137,4 +137,21 @@ describe('Query', () => { expect(console.warn).toHaveBeenCalledTimes(2); expect((console.warn as any).mock.calls[0][0]).toMatch(/writer/); }); + + it('should skip undefined values that are expected', () => { + const query = gql` + { + field + } + `; + + write(store, { query }, { field: 'test' } as any); + // This should not overwrite the field + write(store, { query }, { field: undefined } as any); + // Because of us writing an undefined field + expect(console.warn).toHaveBeenCalledTimes(1); + expect((console.warn as any).mock.calls[0][0]).toMatch(/undefined/); + // The field must still be `'test'` + expect(store.getRecord('Query.field')).toBe('test'); + }); });