Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Fix issue with usage in TypeScript projects caused by 'compose' re-export #291

Merged
merged 1 commit into from
Oct 28, 2016
Merged

Fix issue with usage in TypeScript projects caused by 'compose' re-export #291

merged 1 commit into from
Oct 28, 2016

Conversation

larixer
Copy link
Contributor

@larixer larixer commented Oct 27, 2016

Fixes #279

As suggested by @migueloller compose is re-exported from redux instead of recompose, since implementation is completely the same, that's why we can drop depending on recompose completely.

TODO:

  • If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass
  • Update CHANGELOG.md with your change
  • If this was a change that affects the external API, update the docs and post a link to the PR in the discussion

@@ -2,6 +2,6 @@ import ApolloProvider from './ApolloProvider';
import graphql, { withApollo } from './graphql';

// expose easy way to join queries from recompose

Choose a reason for hiding this comment

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

Update the comment to say redux instead of recompose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Done

@migueloller
Copy link

migueloller commented Oct 27, 2016

Would it make sense to rebase so that the commit that removes the recompose dependency also imports from redux? This way the commit makes a non-breaking atomic change and can easily be reverted if necessary.

EDIT: This won't matter if the PR is squashed.

@larixer
Copy link
Contributor Author

larixer commented Oct 27, 2016

Sure. I will try now. I never did this before.... Any pointers appreciated

@larixer
Copy link
Contributor Author

larixer commented Oct 27, 2016

Okay, rebased everything into single commit

@jbaxleyiii
Copy link
Contributor

@Vlasenko this is great, my only concern is with importing all of redux. This probably isn't an issue since apollo-client imports redux. Technically this integration doesn't actually rely on redux at all.

That being said, I'm great with this for now! Thanks for the PR

@jbaxleyiii jbaxleyiii merged commit 1e4249e into apollographql:master Oct 28, 2016
@jbaxleyiii jbaxleyiii mentioned this pull request Oct 28, 2016
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants