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

[WIP]:Initial Compose functionality and tests #31

Merged
merged 6 commits into from
Jun 23, 2018

Conversation

mringer
Copy link
Contributor

@mringer mringer commented May 14, 2018

#4

Quick and dirty implementation of the compose functionality proposed in the API v2 thread.
With some basic tests.

Proposed Usage:

// This works.
const base = new Composable( resolve => {}, error => {} );
const composed = base.compose( { 
  r1: { resolve: r => {}, error: e =>  }, 
  r2: { resolve: r => {}, error: e =>  },
  r3: resolve => {}
});

// probably need to bump up the ES version to make this work.
const gqlResolvers = { Mutaitons: { ...composed } };

@mringer mringer mentioned this pull request May 14, 2018
@thebigredgeek
Copy link
Owner

thebigredgeek commented May 16, 2018

Couldn't we just add a compose helper similar to and and or? By that, I mean rather than constructing a Composable couldn't we pass a series of resolvers into a compose HoF?

const isHappy = compose(
  isNotSad,
  isNotMad,
  isNotStressed,
  isNotAnxious
)(
(root, args, context, info) => ...
(root, args, context, error, info) => ...
)

@mringer
Copy link
Contributor Author

mringer commented May 16, 2018

Yeah, absolutely. Figured I'd stick strictly to the proposed implementation in the v2 spec, but that's totally reasonable.

@thebigredgeek
Copy link
Owner

thebigredgeek commented May 18, 2018

Oh I see what you did. This is actually quite clever! Let's stick with this approach, but maybe axe the "composable" class requirement?

@mringer
Copy link
Contributor Author

mringer commented May 18, 2018

Yeah, we can make composable a function, rather than a class. It's more consistent with the rest of the API.

@mringer mringer force-pushed the mr-feature-composeApi branch from e10d9f5 to 84696cf Compare May 18, 2018 18:50
@mringer
Copy link
Contributor Author

mringer commented May 18, 2018

@thebigredgeek ok, refactored and added a few more tests.

// The basic API would be: 
const myResolver = createResolver( resolve => {}, error => {} );
const base = composable( resolve => {}, error => {} );

// All composed will pass calls and bubble errors though base.
const composed = base.compose( { 
  r1: { resolve: r => {}, error: e => {}  },  // declate separate resolve and error handlers
  r3: resolve => {}, // resolve declared inline 
  r4: myResolver // reuse a pre-constructed resolver
});

// And provide your Query and Mutation wiring like so...

{ Query: {
  someResolver: resFn,
  ...composed
},
Mutation: {
  someMutation: muteFn,
  ...composed
}

Added babel-plugin-transform-object-rest-spread for the spread operator syntax.

Updated babel-preset-es2015 to babel-preset-env as recommended by babel's docs. (though not strictly necessary for this PR)

@mringer mringer changed the title Initial Compose functionality and tests [WIP]:Initial Compose functionality and tests May 21, 2018
@thebigredgeek
Copy link
Owner

thebigredgeek commented May 23, 2018

Right, sorry... what I meant was that couldn't we just do something like...

const base = createResolver(
   () => undefined,
   err => err
);

// All composed will pass calls and bubble errors though base.
const composed = base.compose( { 
  r1: { resolve: r => {}, error: e => {}  },  // declate separate resolve and error handlers
  r3: resolve => {}, // resolve declared inline 
  r4: myResolver // reuse a pre-constructed resolver
});

// And provide your Query and Mutation wiring like so...

{ Query: {
  someResolver: resFn,
  ...composed
},
Mutation: {
  someMutation: muteFn,
  ...composed
}

So basically just adding base.compose in addition to base.createResolver? Thoughts?

@thebigredgeek
Copy link
Owner

@mringer bump

@mringer
Copy link
Contributor Author

mringer commented Jun 7, 2018

Word, busy couple weeks at work, should be able to circle back to this in the next couple days.

@mringer mringer force-pushed the mr-feature-composeApi branch from 99dd1bb to 3593237 Compare June 13, 2018 19:14
@mringer
Copy link
Contributor Author

mringer commented Jun 13, 2018

@thebigredgeek I think this is what you were asking for.

Copy link
Owner

@thebigredgeek thebigredgeek left a comment

Choose a reason for hiding this comment

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

glorious

@mringer mringer merged commit 7d49d06 into thebigredgeek:master Jun 23, 2018
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