Skip to content

Add @After decorator#56

Merged
marcelorisse merged 4 commits intomasterfrom
feature/after-decorator
Nov 23, 2017
Merged

Add @After decorator#56
marcelorisse merged 4 commits intomasterfrom
feature/after-decorator

Conversation

@marcelorisse
Copy link
Copy Markdown

Copy link
Copy Markdown

@felipesabino felipesabino left a comment

Choose a reason for hiding this comment

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

👏

* ```
* @param option Options for an Schema
*/
export function After(option: AfterOption | AfterMiddleware) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I noticed that this is similar to the @Before decorator implementation and I used the BeforeOption | BeforeMiddleware there just to keep it compatible with previous implementation but I wouldn't mind changing/breaking it to always be just BeforeOption.

Anyway, I guess we can have just AfterOption here to make it similar to other decorator and plan to change the @Before to have just BeforeOption as argument. What do you think?

describe('Pagination', function () {

it('returns a GraphQL Pagination object with custom @OrberBy fields', async function () {
it('returns a GraphQL Pagination object with custom @OrderBy fields', async function () {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oops 😊

Comment thread src/decorator/after.decorator.ts Outdated
*
* Usage example:
* ```
* let middleware: AfterMiddleware = (context: any, args: { [key: string]: any }, next: (error?: Error, value?: any) => any): 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.

I guess it is missing result for the middleware arguments here, isn't it?

Comment thread src/middleware.ts Outdated
@@ -1 +1,8 @@
export type Middleware = (context: any, args: { [key: string]: any }, next: (error?: Error, value?: any) => any) => Promise<any> | 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 please also rename Middleware to BeforeMiddleware?

@marcelorisse marcelorisse merged commit 914e18c into master Nov 23, 2017
@marcelorisse marcelorisse deleted the feature/after-decorator branch November 23, 2017 16:39
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.

2 participants