Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Scalar as argument #65

Closed
ashleyw opened this issue Apr 20, 2018 · 13 comments
Closed

Custom Scalar as argument #65

ashleyw opened this issue Apr 20, 2018 · 13 comments
Labels
Bug 🐛 Something isn't working
Projects
Milestone

Comments

@ashleyw
Copy link

ashleyw commented Apr 20, 2018

Is it possible to use a custom scalar as an argument in a resolver? It appears like convertToType expects a ClassType, but GraphQLScalarType returns an object?

Thanks!

@ashleyw
Copy link
Author

ashleyw commented Apr 20, 2018

Ah! I needed to map the scalar in buildSchema's scalarsMap property, and specify the underlying class as the @Arg type, rather than the GraphQLScalarType

@ashleyw ashleyw closed this as completed Apr 20, 2018
@MichalLytek
Copy link
Owner

Yes, you can provide the custom scalar to the @Arg decorator or map it with the class if custom scalar convert from/to class (like Date or ObjectId).

class SampleResolver {
  @Query()
  sampleQuery(@Arg(type => CustomScalar) data: any): boolean {
    return true;
  }
}

@ashleyw
Copy link
Author

ashleyw commented Apr 20, 2018

@19majkel94 are you sure the former is true? In the convertToType method it tries to call new CustomScalar(), where CustomScalar isn't a function/class but instead an object provided by GraphQLScalarType.

@MichalLytek
Copy link
Owner

For getting query/mutation/resolver arg, it calls getGraphQLInputType, which calls convertTypeIfScalar, which checks:

if (type instanceof GraphQLScalarType) {
  return type;
}

@ashleyw
Copy link
Author

ashleyw commented Apr 24, 2018

That's strange. I've just experienced this same issue again, check this out:

Custom Scalar:

import { GraphQLScalarType, Kind } from 'graphql';
import { PaginationDirection } from '../input/PaginationArgs';

export interface CursorID {
  id: number | string;
  direction: PaginationDirection;
}

export interface CursorPaged {
  skip: number;
}

export type Cursor = CursorID | CursorPaged;

export const CursorScalar = new GraphQLScalarType({
  name: 'Cursor',
  description: 'Pagination cursor',

  parseValue(value: string): Cursor {
    return JSON.parse(Buffer.from(value, 'base64').toString('ascii'));
  },

  serialize(value: Cursor) {
    return Buffer.from(JSON.stringify(value)).toString('base64');
  },

  parseLiteral(ast): Cursor | null {
    if (ast.kind === Kind.STRING) {
      return JSON.parse(Buffer.from(ast.value, 'base64').toString('ascii'));
    }
    return null;
  },
});

Argument in my resolver:

@Arg('cursor', type => CursorScalar, { nullable: true })
cursor: Cursor

Error:

TypeError: Target is not a constructor
    at Object.convertToType (/app/node_modules/type-graphql/helpers/types.js:52:26)
    at Object.<anonymous> (/app/node_modules/type-graphql/resolvers/helpers.js:22:69)
    at Generator.next (<anonymous>)
    at /app/node_modules/type-graphql/resolvers/helpers.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/type-graphql/resolvers/helpers.js:3:12)
    at Promise.all.params.sort.map (/app/node_modules/type-graphql/resolvers/helpers.js:17:88)
    at Array.map (<anonymous>)
    at Object.<anonymous> (/app/node_modules/type-graphql/resolvers/helpers.js:17:69)
    at Generator.next (<anonymous>)
    at /app/node_modules/type-graphql/resolvers/helpers.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/type-graphql/resolvers/helpers.js:3:12)
    at Object.getParams (/app/node_modules/type-graphql/resolvers/helpers.js:16:12)
    at /app/node_modules/type-graphql/resolvers/create.js:23:44
    at Generator.next (<anonymous>)

Am I doing something wrong @19majkel94 ?

@ashleyw
Copy link
Author

ashleyw commented Apr 24, 2018

There's a discrepancy between GraphQLScalarType used in my project and type-graphql.

  const schema = await TypeGraphQL.buildSchema({
    resolvers: [`${__dirname}/resolvers/**/*.ts`],
    scalarsMap: [{ type: Cursor, scalar: CursorScalar }],
  });

This results in a type warning:

Type 'GraphQLScalarType' is not assignable to type 'GraphQLScalarType'. Two different types with this name exist, but they are unrelated.
    Property 'toJSON' is missing in type 'GraphQLScalarType'.

Which is strange, since my package.json defines version ^0.13.2 of graphql (the same as type-graphql.)

@MichalLytek
Copy link
Owner

MichalLytek commented Apr 24, 2018

Try npm dedupe to flatten the tree and use the same module.
For me looks like the discrepancy is the source of this problem.

Maybe I should hide graphql-js and just export GraphQLScalarType and other from type-graphql.

@ashleyw
Copy link
Author

ashleyw commented Apr 24, 2018

Upgrading my @types/graphql package removed the warning, but I'm still experiencing the issue. The type instanceof GraphQLScalarType check you mentioned actually resolves as true.

@ashleyw ashleyw reopened this Apr 24, 2018
@MichalLytek
Copy link
Owner

Can you compile the app using tsc and see what TS reflection generates for this in your app:

@Arg('cursor', type => CursorScalar, { nullable: true })
cursor: Cursor

It should look something like this:

__decorate([
    src_1.Query(returns => [recipe_type_1.Recipe]),
    __metadata("design:type", Function),
    __metadata("design:paramtypes", []),
    __metadata("design:returntype", Promise)
], ExampleResolver.prototype, "recipes", null);
__decorate([
    src_1.Authorized() // only logged users can add new recipe
    ,
    src_1.Mutation(),
    __param(0, src_1.Arg("title")),
    __param(1, src_1.Arg("description", { nullable: true })),
    __metadata("design:type", Function),
    __metadata("design:paramtypes", [String, String]),
    __metadata("design:returntype", recipe_type_1.Recipe)
], ExampleResolver.prototype, "addRecipe", null);
__decorate([
    src_1.Authorized("ADMIN") // only admin can remove the published recipe
    ,
    src_1.Mutation(),
    __param(0, src_1.Arg("title")),
    __metadata("design:type", Function),
    __metadata("design:paramtypes", [String]),
    __metadata("design:returntype", Boolean)
], ExampleResolver.prototype, "deleteRecipe", null);
ExampleResolver = __decorate([
    src_1.Resolver()
], ExampleResolver);
exports.ExampleResolver = ExampleResolver;

I wounder to what the Cursor type is reflected and why it can't be newable. I could add this to exclusion list.

@ashleyw
Copy link
Author

ashleyw commented Apr 24, 2018

__decorate([
    type_graphql_1.FieldResolver(),
    __param(0, type_graphql_1.Root()),
    __param(1, type_graphql_1.Arg('cursor', type => Cursor_1.CursorScalar, { nullable: true })),
    __param(2, type_graphql_1.Arg('limit', { nullable: true })),
    __param(3, type_graphql_1.Arg('direction', type => PaginationArgs_1.PaginationDirection, { nullable: true })),
    __metadata("design:type", Function),
    __metadata("design:paramtypes", [University_1.University, Object, Number, String]),
    __metadata("design:returntype", Promise)
], UniversityResolver.prototype, "technologies", null);

I'm fairly new to Typescript so I'm not sure exactly what I'm looking at. Would it be the Object in design:paramtypes?

I've added Object to simpleTypes and it works. But I assume there's a reason why Object isn't already in that list? Are custom scalars allowed to be simple interfaces/objects?

@MichalLytek
Copy link
Owner

Object might be ignored but it works:

PS C:\WINDOWS\system32> node
> const o = new Object()
undefined
> o
{}
> o.test = "test"
'test'
> o
{ test: 'test' }
>

Unfortunately I don't know from the stacktrace on which argument it failed. Can you create a minimal reproducible example for that case? I would then be able to debug it and fix 😉

@MichalLytek MichalLytek added the Bug 🐛 Something isn't working label Apr 24, 2018
@ashleyw
Copy link
Author

ashleyw commented Apr 24, 2018

Here you go: https://github.com/ashleyw/type-graphql-issue-65-example :)

@MichalLytek
Copy link
Owner

MichalLytek commented Apr 24, 2018

Ok, now I see that. Your scalar parse returns custom object, which goes through the if that checks the type of data only, not the target type:

if (simpleTypes.includes(data.constructor)) {
  return data;
}

Fixed in 4176ed2 🎉
If you need transforming scalar object to class, use scalarsMap feature 😉

@MichalLytek MichalLytek added this to Backlog in Board via automation Oct 22, 2018
@MichalLytek MichalLytek added this to the 1.0.0 release milestone Oct 22, 2018
@MichalLytek MichalLytek moved this from Backlog to Done in Board Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working
Projects
Board
  
Done
Development

No branches or pull requests

2 participants