Skip to content

Commit

Permalink
fix(types): stop converting objects that are instances of the class
Browse files Browse the repository at this point in the history
  • Loading branch information
MichalLytek committed Feb 9, 2019
1 parent 315d7d5 commit 7bddb04
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- fix calling return type getter function `@Field(type => Foo)` before finishing module evaluation (allow for extending circular classes using `require`)
- fix nullifying other custom method decorators - call the method on target instance, not the stored reference to original function (#247)
- fix throwing error when extending non args class in the `@ArgsType()` class
- prevent unnecessary conversion of an object that is already an instance of the requested type (avoid constructor side-effects)

## v0.16.0
### Features
Expand Down
4 changes: 4 additions & 0 deletions src/helpers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ export function convertToType(Target: any, data?: object): object | undefined {
if (simpleTypes.includes(data.constructor)) {
return data;
}
// skip converting already converted types
if (data instanceof Target) {
return data;
}

return Object.assign(new Target(), data);
}
Expand Down
23 changes: 23 additions & 0 deletions tests/functional/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,7 @@ describe("Resolvers", () => {
let queryContext: any;
let queryInfo: any;
let descriptorEvaluated: boolean;
let sampleObjectConstructorCallCount: number;

function DescriptorDecorator(): MethodDecorator {
return (obj, methodName, descriptor: any) => {
Expand Down Expand Up @@ -1119,6 +1120,7 @@ describe("Resolvers", () => {
queryContext = undefined;
queryInfo = undefined;
descriptorEvaluated = false;
sampleObjectConstructorCallCount = 0;
});

beforeAll(async () => {
Expand Down Expand Up @@ -1156,6 +1158,9 @@ describe("Resolvers", () => {
isTrue() {
return this.TRUE;
}
constructor() {
sampleObjectConstructorCallCount++;
}

instanceValue = Math.random();

Expand Down Expand Up @@ -1427,6 +1432,24 @@ describe("Resolvers", () => {
expect(getterFieldResult1).not.toEqual(getterFieldResult2);
});

it("shouldn't create new instance for object type if it's already an instance of its class", async () => {
const query = /* graphql */ `
query {
sampleQuery {
getterField
methodField
}
}
`;

const result = await graphql(schema, query);
const getterFieldValue = result.data!.sampleQuery.getterField;
const methodFieldValue = result.data!.sampleQuery.getterField;

expect(getterFieldValue).toEqual(methodFieldValue);
expect(sampleObjectConstructorCallCount).toBe(1);
});

it("should use the same instance of resolver class for consecutive queries", async () => {
const query = `query {
sampleQuery {
Expand Down

0 comments on commit 7bddb04

Please sign in to comment.