Skip to content
This repository has been archived by the owner on Aug 20, 2020. It is now read-only.

Extract gql extraction into another package #19

Open
lewisf opened this issue Mar 20, 2017 · 10 comments
Open

Extract gql extraction into another package #19

lewisf opened this issue Mar 20, 2017 · 10 comments

Comments

@lewisf
Copy link

lewisf commented Mar 20, 2017

The GQL extraction provided in persistgraphql is pretty great. I'm hoping to use it to add support for javascript source files within apollo-codegen (apollographql/apollo-tooling#25) or similar projects and having the gql extraction stuff available as a separate package would make that much easier. It that something that sounds reasonable?

@Poincare
Copy link
Contributor

Yeah, definitely! In fact, that would help solve #17 as well since people could just build stuff on top of the ExtractGQL class or similar. Would be happy to accept PRs on this and also get involved in the implementation.

@lewisf
Copy link
Author

lewisf commented Mar 20, 2017

awesome, this is something that im happy to take a look at in the next week or so.

where should this go? should an apollographql/extract-gql repo be created for this? or should i just create a repo to do this and transfer it over later?

@stubailo
Copy link
Contributor

@lewisf I think the best option is for you to create a prototype on a personal repo, then we can move it into the organization afterwards?

We should also see if this can replace https://github.com/apollographql/graphql-document-collector, which is pretty old and I'm not sure many people use it. The intention was to do something similar.

@lewisf
Copy link
Author

lewisf commented Mar 23, 2017

Okay, sounds reasonable, I'll give it a shot

@lewisf
Copy link
Author

lewisf commented Mar 26, 2017

Quick update after speaking with @Poincare. It looks like the ExtractGQL class is exactly what should get extracted.

After extracting it we'd be able to use it to solve #17 as mentioned above, and I'd be able to use it to address apollographql/apollo-tooling#25.

I'm going to work on this and seeing if anything needs to change (public/private methods) etc. We can leave this issue open until that package exists and is integrated back into persistgraphql.

@lewisf
Copy link
Author

lewisf commented Mar 27, 2017

Okay, I've done some initial work in extracting ExtractGQL here:
https://github.com/lewisf/extract-gql

I've even tried using it in another project locally and it worked well (yay). I haven't done an audit of the public interface but it's good to know that the extraction was pretty straightforward due to how ExtractGQL was written here.

Some key improvements I want to make to extract-gql before publishing it as a package are going to be documented there. Namely:

Please let me know if there are any other issues that should be addressed before extract-gql should go live.

I've also opened up other issues on it based on me trying to use it with an existing project we have here at Coursera, but I don't think those are blocking the re-integration of extract-gql back into persistgraphql.

@Poincare
Copy link
Contributor

@lewisf Great! Most of those issues and general direction look good to me.

However, I did think that the package you would be building wouldn't actually include any of the CLI portions that this tool provides. Instead, it should just focus on providing a simple library that allows other projects (e.g. apollo-codegen) to build on top of the query extraction. Then, persistgraphql can simply use that library internally rather than having the ExtractGQL class as it is currently implemented and persistgraphql would provide the CLI functionality on top. Does that sound reasonable?

@lewisf
Copy link
Author

lewisf commented Mar 30, 2017

@Poincare yes that definitely does sound reasonable. I'll go ahead and make that modification to extract-gql's responsibilities

@Poincare
Copy link
Contributor

Hey @lewisf any progress on this? Any way I can help?

@lewisf
Copy link
Author

lewisf commented May 26, 2017

@Poincare sorry! i had to shift focuses to prioritize something else -- happy to chat about this if you wanted to take a stab at this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants