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

Import GraphQL files from other GraphQL files (closes #1477) #1892

Merged
merged 2 commits into from Aug 14, 2018
Merged

Import GraphQL files from other GraphQL files (closes #1477) #1892

merged 2 commits into from Aug 14, 2018

Conversation

arianon
Copy link
Contributor

@arianon arianon commented Aug 13, 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.

@@ -1,15 +1,62 @@
const Asset = require('../Asset');
const localRequire = require('../utils/localRequire');
const Resolver = require('../Resolver');
const fs = require('../utils/fs');
Copy link
Member

Choose a reason for hiding this comment

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

Please use parcels fs util

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty sure I am?

Copy link
Member

Choose a reason for hiding this comment

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

Damn, not sure why I thought it wasn't


await Promise.all(
code
.split(os.EOL)
Copy link
Member

Choose a reason for hiding this comment

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

Will this work in every case?
Seems like this would not work if a user clones a repo that is being used developed various OSes

Copy link
Contributor Author

@arianon arianon Aug 14, 2018

Choose a reason for hiding this comment

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

Good point, using a regex such as /\r?\n/ is probably more resilient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to use /\r\n?|\n which is pretty much the same as what the GraphQL specification defines as a line terminator

@DeMoorJasper DeMoorJasper merged commit afbca87 into parcel-bundler:master Aug 14, 2018
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.

None yet

2 participants