enhance: Union types construct class instances in ctrl.fetch() resolu…

…tion value (#3063)

.changeset/fuzzy-cherries-provide.md

@@ -0,0 +1,12 @@
+---
+"@data-client/endpoint": patch
+"@data-client/graphql": patch
+"@data-client/rest": patch
+---
+
+Polymorphic (Union) types should still denormalize when handling passthrough (non-normalized) data
+
+When denormalizing non-normalized (like return of ctrl.fetch), it is still expected to handle
+all steps like constructing class instances if possible. However, to do this for Polymorphic
+types we need to fallback to using part of the normalize process to find out *which* schema
+to use for the remainder of denormalization.

docs/rest/api/Array.md

@@ -31,7 +31,7 @@ For unbounded collections with `string` keys, use [schema.Values](./Values.md)
 
 :::tip
 
-Make it mutable with [Collections](./Collection.md)
+Make it mutable (new items can be [pushed](./Collection.md#push)/[unshifted](./Collection.md#unshift)) with [Collections](./Collection.md)
 
 :::
 

docs/rest/api/Values.md

@@ -24,7 +24,7 @@ Describes a map whose values follow the given schema.
 
 :::tip
 
-Make it mutable with [Collections](./Collection.md)
+Make it mutable (new items can be [assigned](./Collection.md#assign)) with [Collections](./Collection.md)
 
 :::
 

packages/endpoint/src/schemas/Polymorphic.ts

@@ -104,6 +104,12 @@ Value: ${JSON.stringify(value, undefined, 2)}`,
       value &&
       (isImmutable(value) ? value.get('schema') : value.schema);
     if (!this.isSingleSchema && !schemaKey) {
+      // denormalize should also handle 'passthrough' values (not normalized) and still
+      // construct the correct Entity instance
+      if (typeof value === 'object' && value !== null) {
+        const schema = this.inferSchema(value, undefined, undefined);
+        if (schema) return unvisit(value, schema);
+      }
       /* istanbul ignore else */
       if (process.env.NODE_ENV !== 'production' && value) {
         console.warn(

packages/endpoint/src/schemas/__tests__/Union.test.js

@@ -336,7 +336,11 @@ describe.each([
             groups: Group,
           },
           input => {
-            return input.username ? 'users' : 'groups';
+            return (
+              input.username ? 'users'
+              : input.groupname ? 'groups'
+              : undefined
+            );
           },
         );
 
@@ -353,7 +357,11 @@ describe.each([
             groups: Group,
           },
           input => {
-            return input.username ? 'users' : 'groups';
+            return (
+              input.username ? 'users'
+              : input.groupname ? 'groups'
+              : undefined
+            );
           },
         );
 
@@ -370,7 +378,11 @@ describe.each([
             groups: Group,
           },
           input => {
-            return input.username ? 'users' : 'groups';
+            return (
+              input.username ? 'users'
+              : input.groupname ? 'groups'
+              : undefined
+            );
           },
         );
 
@@ -384,7 +396,11 @@ describe.each([
             groups: Group,
           },
           input => {
-            return input.username ? 'users' : 'groups';
+            return (
+              input.username ? 'users'
+              : input.groupname ? 'groups'
+              : undefined
+            );
           },
         );
 

packages/react/src/__tests__/integration-endpoint.web.tsx

@@ -427,6 +427,12 @@ describe.each([
     const prevWarn = global.console.warn;
     global.console.warn = jest.fn();
 
+    const response = [
+      null,
+      { id: '5', body: 'hi', type: 'first' },
+      { id: '5', body: 'hi', type: 'another' },
+      { id: '5', body: 'hi' },
+    ];
     const { result } = renderDataClient(
       () => {
         return useSuspense(UnionResource.getList);
@@ -436,12 +442,7 @@ describe.each([
           {
             endpoint: UnionResource.getList,
             args: [],
-            response: [
-              null,
-              { id: '5', body: 'hi', type: 'first' },
-              { id: '5', body: 'hi', type: 'another' },
-              { id: '5', body: 'hi' },
-            ],
+            response,
           },
         ],
       },
@@ -451,6 +452,9 @@ describe.each([
     expect(result.current[1]).toBeInstanceOf(FirstUnion);
     expect(result.current[2]).not.toBeInstanceOf(FirstUnion);
     expect(result.current[3]).not.toBeInstanceOf(FirstUnion);
+    // should still passthrough objects
+    expect(result.current[2]).toEqual(result.current[2]);
+    expect(result.current[3]).toEqual(result.current[3]);
     expect((global.console.warn as jest.Mock).mock.calls).toMatchSnapshot();
     global.console.warn = prevWarn;
   });

packages/react/src/hooks/__tests__/useController/__snapshots__/fetch.tsx.snap

@@ -6,8 +6,86 @@ exports[`CacheProvider should log error message when user update method throws 1
 ]
 `;
 
+exports[`CacheProvider should return denormalized value when schema is present (unions) 1`] = `
+[
+  [
+    "Schema attribute "another" is not expected.
+Expected one of: "first", "second"
+
+Value: {
+  "id": "6",
+  "body": "hi",
+  "type": "another"
+}",
+  ],
+  [
+    "Schema attribute undefined is not expected.
+Expected one of: "first", "second"
+
+Value: {
+  "id": "7",
+  "body": "hi"
+}",
+  ],
+  [
+    "TypeError: Unable to infer schema for UnionSchema
+Value: {
+  "id": "6",
+  "body": "hi",
+  "type": "another"
+}.",
+  ],
+  [
+    "TypeError: Unable to infer schema for UnionSchema
+Value: {
+  "id": "7",
+  "body": "hi"
+}.",
+  ],
+]
+`;
+
 exports[`ExternalCacheProvider should log error message when user update method throws 1`] = `
 [
   "The following error occured during Endpoint.update() for POST http://test.com/article-cooler",
 ]
 `;
+
+exports[`ExternalCacheProvider should return denormalized value when schema is present (unions) 1`] = `
+[
+  [
+    "Schema attribute "another" is not expected.
+Expected one of: "first", "second"
+
+Value: {
+  "id": "6",
+  "body": "hi",
+  "type": "another"
+}",
+  ],
+  [
+    "Schema attribute undefined is not expected.
+Expected one of: "first", "second"
+
+Value: {
+  "id": "7",
+  "body": "hi"
+}",
+  ],
+  [
+    "TypeError: Unable to infer schema for UnionSchema
+Value: {
+  "id": "6",
+  "body": "hi",
+  "type": "another"
+}.",
+  ],
+  [
+    "TypeError: Unable to infer schema for UnionSchema
+Value: {
+  "id": "7",
+  "body": "hi"
+}.",
+  ],
+]
+`;

packages/react/src/hooks/__tests__/useController/fetch.tsx

@@ -1,7 +1,13 @@
 import { ResolveType } from '@data-client/core';
 import { CacheProvider } from '@data-client/react';
 import { CacheProvider as ExternalCacheProvider } from '@data-client/redux';
-import { CoolerArticle, FutureArticleResource } from '__tests__/new';
+import {
+  CoolerArticle,
+  CoolerArticleResource,
+  FirstUnion,
+  FutureArticleResource,
+  UnionResource,
+} from '__tests__/new';
 import nock from 'nock';
 
 import { useCache, useSuspense } from '../..';
@@ -105,6 +111,13 @@ describe.each([
   afterEach(() => {
     errorspy.mockRestore();
   });
+  let warnSpy: jest.SpyInstance;
+  afterEach(() => {
+    warnSpy.mockRestore();
+  });
+  beforeEach(() =>
+    (warnSpy = jest.spyOn(console, 'warn')).mockImplementation(() => {}),
+  );
 
   it('should fetch', async () => {
     const { result } = renderDataClient(() => {
@@ -293,4 +306,56 @@ describe.each([
     expect(throws.length).toBe(1);
     expect(result.current[0]).toBe(null);
   });
+
+  it('should return denormalized value when schema is present', async () => {
+    const { controller } = renderDataClient(
+      () => {
+        return 'hi';
+      },
+      {
+        resolverFixtures: [
+          {
+            endpoint: CoolerArticleResource.get,
+            args: [{ id: payload.id }],
+            response: payload,
+          },
+        ],
+      },
+    );
+    const ret = await controller.fetch(CoolerArticleResource.get, {
+      id: payload.id,
+    });
+    expect(ret.content).toEqual(payload.content);
+    expect(ret).toBeInstanceOf(CoolerArticle);
+    expect(warnSpy.mock.calls.length).toBe(0);
+  });
+
+  it('should return denormalized value when schema is present (unions)', async () => {
+    const response = [
+      null,
+      { id: '5', body: 'hi', type: 'first' },
+      { id: '6', body: 'hi', type: 'another' },
+      { id: '7', body: 'hi' },
+    ];
+    const { controller } = renderDataClient(
+      () => {
+        return 'hi';
+      },
+      {
+        resolverFixtures: [
+          {
+            endpoint: UnionResource.getList,
+            args: [],
+            response,
+          },
+        ],
+      },
+    );
+    const ret = await controller.fetch(UnionResource.getList);
+    expect(ret[0]).toBeNull();
+    expect(ret[1]).toBeInstanceOf(FirstUnion);
+    expect(ret[2]).toEqual(response[2]);
+    expect(ret[3]).toEqual(response[3]);
+    expect(warnSpy.mock.calls).toMatchSnapshot();
+  });
 });