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

Use graphql-tag webpack loader code to allow using #import for fragment #1817

Closed
wants to merge 3 commits into from

Conversation

nabiltntn
Copy link

Allow using the #import directive to import fragments inside .graphql files

I reused the code of graphql-tag webpack loader that parses .graphql files to generate modules that include fragments with queries definitions.

This PR addresses issue #1477

Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

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

Either change the code to not be an exact copy, or extract this into functions with a note above mentioning the origin, as this is in violation with MIT license

@nabiltntn
Copy link
Author

I removed the copied code and i used loader function directly from 'graphql-tag' package.

Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

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

Not sure if requiring an internal piece of code from a webpack plugin is such a great idea.

Would probably be better to copy it into a seperate file inside parcel or inside the grapQL asset with proper credits to the original author.

I'm also pretty sure it's not using the parcel resolver to resolve files, which could be problematic in the future.

DeMoorJasper pushed a commit that referenced this pull request Aug 14, 2018
An alternative to #1817 that doesn't rely on using `graphql-tag/loader`, which is a Webpack loader.

#### What this does:
1. Recursively traverse the GraphQL files.
2. Collect all the imports into a map.
3. Concatenate them.
4. Parse the result into an AST.

#### What this doesn't do:

This PR (as of time of writing) does not have feature parity with `graphql-tag/loader`, meaning that:
* It does not de-duplicate fragments that have the same name. _This is a behavior that I personally disapprove of since it can lead to surprising results instead of throwing an error if a user accidentally (however unlikely) reuses an already defined fragment name._
* It does not collect multiple defined queries/mutations and make them individually require-able from a JavaScript parent.


If the above behaviours are desired then I can implement them.
devongovett pushed a commit that referenced this pull request Oct 15, 2018
An alternative to #1817 that doesn't rely on using `graphql-tag/loader`, which is a Webpack loader.

#### What this does:
1. Recursively traverse the GraphQL files.
2. Collect all the imports into a map.
3. Concatenate them.
4. Parse the result into an AST.

#### What this doesn't do:

This PR (as of time of writing) does not have feature parity with `graphql-tag/loader`, meaning that:
* It does not de-duplicate fragments that have the same name. _This is a behavior that I personally disapprove of since it can lead to surprising results instead of throwing an error if a user accidentally (however unlikely) reuses an already defined fragment name._
* It does not collect multiple defined queries/mutations and make them individually require-able from a JavaScript parent.


If the above behaviours are desired then I can implement them.
devongovett pushed a commit that referenced this pull request Oct 15, 2018
An alternative to #1817 that doesn't rely on using `graphql-tag/loader`, which is a Webpack loader.

#### What this does:
1. Recursively traverse the GraphQL files.
2. Collect all the imports into a map.
3. Concatenate them.
4. Parse the result into an AST.

#### What this doesn't do:

This PR (as of time of writing) does not have feature parity with `graphql-tag/loader`, meaning that:
* It does not de-duplicate fragments that have the same name. _This is a behavior that I personally disapprove of since it can lead to surprising results instead of throwing an error if a user accidentally (however unlikely) reuses an already defined fragment name._
* It does not collect multiple defined queries/mutations and make them individually require-able from a JavaScript parent.


If the above behaviours are desired then I can implement them.
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