-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Import GraphQL files from other GraphQL files (closes #1477) #1892
Conversation
@@ -1,15 +1,62 @@ | |||
const Asset = require('../Asset'); | |||
const localRequire = require('../utils/localRequire'); | |||
const Resolver = require('../Resolver'); | |||
const fs = require('../utils/fs'); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure I am?
There was a problem hiding this comment.
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
src/assets/GraphqlAsset.js
Outdated
|
||
await Promise.all( | ||
code | ||
.split(os.EOL) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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.
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.
An alternative to #1817 that doesn't rely on using
graphql-tag/loader
, which is a Webpack loader.What this does:
What this doesn't do:
This PR (as of time of writing) does not have feature parity with
graphql-tag/loader
, meaning that:If the above behaviours are desired then I can implement them.