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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use graphql-tag in a pragmatic way #5

Merged
merged 3 commits into from May 3, 2017

Conversation

Projects
None yet
3 participants
@xavxyz
Copy link
Contributor

xavxyz commented Apr 26, 2017

Fixes issues #1, #2, #3.

馃暪 Current state

The current state of jest-transform-graphql depends explicitly on an outdated version of graphql-tag and run basically this version of gql on source files.

馃毀 Problem

We cannot use "evolved" graphql syntax in our files (such as imports) and are tied to an outdated version of graphql-tag.

鉁堬笍 Straightforward solution

Replace the process part of this transform by the actual code of graphql-tag/loader, such as shown in #2 (comment) or proposed in #4.

This code works well and is about nearly the same as graphql-tag/loader, except that we remove the cacheable option used by Webpack.

馃殌 Pragmatic solution

tldr; DRY.

We can move the latest graphql-tag version as a peer dependency and require directly the loader from it. We then call the loader on the source file with a stubbed context so that the cacheable function exists.

We do not rewrite the graphql-tag code and leverages it at its best 馃槉 Thoughts? 馃挱

xavxyz added some commits Apr 26, 2017

operate the transformation thanks to the loader code directly
* note: atm, graphql-tag makes the result of the parsing cacheable, jest doesn't know about that, so we create a fake context with loader.call(newCtx)
@turadg

This comment has been minimized.

Copy link
Contributor

turadg commented May 3, 2017

Awesome! Thanks for this contribution and thorough description.

@turadg turadg merged commit 34c1e95 into remind101:master May 3, 2017

@paradite

This comment has been minimized.

Copy link

paradite commented Nov 9, 2018

Just a note that this PR creates some complexity for monorepos. For example, with the following setup:

Hoisted dependency (./packages/):

  • jest-transform-graphql
    • graphql-tag (missing)

Package dependency (./packages/abc/):

  • apollo-client
    • graphql-tag

jest would throw errors for missing graphql-tag since it cannot find graphql-tag in the hoisted directory ./packages/.
This issue is masked in normal setup since other graphql libraries such as apollo-client has an dependency on graphql-tag, but using monorepo with hoisting reveals this issue.

I don't know if we should somehow fix it, given that the walkaround is pretty easy (declare graphql-tag separately as hoisted dependency).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.