Skip to content

Decorators less verbose and subscriptions#14

Merged
felipesabino merged 29 commits intoindigotech:masterfrom
playerx:master
Oct 24, 2017
Merged

Decorators less verbose and subscriptions#14
felipesabino merged 29 commits intoindigotech:masterfrom
playerx:master

Conversation

@playerx
Copy link
Copy Markdown

@playerx playerx commented Oct 20, 2017

@playerx
Copy link
Copy Markdown
Author

playerx commented Oct 20, 2017

How to use:

@ObjectType({ description: 'A user type.' })
class User {
    @Field({ type: graphql.GraphQLID }) id: string;
    @Field() name: string;
    @Field() email: string;
}
@InputObjectType({ description: 'Test 123123' })
class UserForUpdate {
    @Field() name: string;
    @Field() email: string;
}

Comment thread src/decorator.ts Outdated
type?: any;
}

export interface ListOption {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since all these description fileds mean the same thing and it is part of every available option object, what if you'd extract them to an interface and then you'd extend that interface from all those options?

Something like:

export interface DefaultOption {
  description?: string;
}

export interface ObjectTypeOption extends DefaultOption { }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I thought about that also but kept it for next step, I was thinking about refactoring current code base, for now everything (about decorators) is in one file and maybe it would be good to split it in separate files, file per decorator + 1 common file.

Comment thread src/decorator.ts

export interface FieldOption {
type?: any;
description?: string;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the pagination? It would be nice to have it here as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree, but first I've implemented just description field to get your feedback and pagination can be implemented with the same way later.

@thiago-soliveira
Copy link
Copy Markdown

Two more things apart from that

  • Could you please write some tests covering this new scenario?
  • You'd be really nice if could mention this new approach on docs.

@playerx
Copy link
Copy Markdown
Author

playerx commented Oct 21, 2017

Okey, I'll need some time and I'll make those things

@playerx
Copy link
Copy Markdown
Author

playerx commented Oct 21, 2017

  • covered new scenarios with tests
  • added pagination, list options in Field decorator
  • added description option for Enum and Value decorators
  • updated documentation
  • created new sample project using Apollo server

@thiago-soliveira check out latest updates please

@playerx
Copy link
Copy Markdown
Author

playerx commented Oct 21, 2017

Also its important your feedback about new sample project structure

@playerx
Copy link
Copy Markdown
Author

playerx commented Oct 22, 2017

We have cool updates, I've added subscription implementation, and created demo in Apollo-server example

Copy link
Copy Markdown

@thiago-soliveira thiago-soliveira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice changes. The server example will help a lot on development and for users as an example.
+1 for the subscription as well.
Let's see how this new structure goes in real life. I think the code got more readable after these changes.

Comment thread src/schema_factory.ts Outdated
const fields: {[key: string]: any} = {};
let mutationTypeFn: Function;
let fieldMetadataList: FieldTypeMetadata[];
// let result: any = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove this code that is commented out?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@felipesabino felipesabino merged commit af9c1db into indigotech:master Oct 24, 2017
@felipesabino felipesabino changed the title Make decorators less verbose Decorators less verbose and subscriptions Oct 24, 2017
@felipesabino
Copy link
Copy Markdown

@playerx Thanks a lot for the contribution! Just published 0.7.0 with these changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants